mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Address Jesper Schulz-Wedde's review on PR #156
- Rename 3 articles so their .good.al/.bad.al companion stems match (do-not-change-primary-key, testfield-required-setup-field, al-identifiers-english), fixing the R14 orphan-sample errors. - do-not-change-primary-key.good.al: include Flow in the new table's own primary key so it actually models the discriminating dimension. - al-build-output-must-not-pollute-project-root.md: drop the unsubstantiated AL0197 causal claim and the non-existent al.outputPath setting; reframe as build-artifact hygiene sourced from ALTool --outfolder / al_build outputPath. - prefer-email-module.md: Email Message is Codeunit 8904, not a table; distinguish it from the underlying Sent/Outbox/Draft storage. - file-datatype-saas.md: File.Open/Create/Read/Write fails to compile against a Cloud-scoped project, it does not compile and silently fail at runtime. - namespace-must-be-verified-from-source.md: narrow to "resolve from the referenced object's source or symbols," since source-file line one is not the only authoritative source (symbol packages, comments before the namespace line). - test-data-must-be-random-and-complete.md: drop "assume an empty database" and "collision-free" absolutes; reframe around independence from unrelated business records and reserving explicit values for scenario-defining inputs. - binary-choice-must-be-boolean.md: scope to genuine true/false semantics, not mechanical two-member-enum-to-boolean conversion. - document-report-word-layout.md: scope down to a sourced Microsoft Learn recommendation instead of an unconditional performance guarantee; cite the three Learn pages. - Wire the new articles into their review skills' candidate-selection signals (file-datatype-saas, prefer-email-module, namespace-must-be-verified-from-source, var-parameters-require-an- addressable-variable) so they can actually enter a worklist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
057e17c202
commit
cc7c1f2ee0
14 changed files with 32 additions and 21 deletions
|
|
@ -1,24 +1,24 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: style
|
||||
keywords: [build, output, alpackages, duplicate, language-server, app-package, project-root, al0197]
|
||||
keywords: [build, output, alpackages, artifact-hygiene, outfolder, project-root]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Keep AL Build Output Out of the Project Root
|
||||
# Write AL Build Artifacts to an Intentional Output Location
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
When an AL project is built, the compiled `.app` file is placed in the project root by default. Over successive builds, multiple `.app` files accumulate there (e.g. one per version). The AL language server, both in the editor and in build tooling, scans the project folder for symbol packages and can load these compiled artefacts alongside the live source files, which produces `AL0197` duplicate-object errors for every object in the project — with messages that point at source lines rather than at the packaged artefact that is the actual duplicate. The errors are not real; they disappear as soon as the stale `.app` files are removed from the root.
|
||||
An AL project's compiled `.app` file can be written to the project root by default, and current tooling explicitly supports choosing a different destination instead — `ALTool`'s `--outfolder` option and the `al_build` agent tool's `outputPath` parameter both exist for this. The problem this rule addresses is not that root-level output is technically invalid; it is agents leaving generated `.app` files scattered through arbitrary source locations, or treating a compiled artefact as if it were part of the source tree (committing it, editing around it, referencing it as a dependency by hand).
|
||||
|
||||
## Best Practice
|
||||
|
||||
Configure the build output path to a dedicated subfolder that is excluded from language server scanning — for example by setting `al.outputPath` to a folder such as `.output` in `.vscode/settings.json`, or by passing an explicit output path to the build tool being used — and add that folder to `.gitignore`. Before treating an `AL0197` "already declared" error as a source code problem, check the project root for stale `.app` files first; adding root `.app` files to `.gitignore` instead of relocating the output path only hides the accumulation rather than fixing it.
|
||||
Write build artifacts to a deliberate, dedicated output location — configured via the build tool actually in use (e.g. `ALTool --outfolder`, or an explicit `outputPath` on the agent build tool) — and add that folder to `.gitignore`. Treat a compiled `.app` as a build artifact, never as a source file to commit or hand-edit around.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Letting `.app` files accumulate in the project root across builds, then debugging the resulting `AL0197` duplicate-object errors as if they were a source code defect instead of first checking for stale build artefacts in the root folder.
|
||||
Letting `.app` files accumulate in arbitrary or unversioned locations without a deliberate output path, or committing compiled artefacts into source control alongside the AL files that produced them.
|
||||
|
|
|
|||
|
|
@ -13,11 +13,11 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
When a field or variable represents exactly two states — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same two-state choice as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code, and it invites a multi-branch check where a simple `if X then` would do. This is distinct from a genuine multi-value choice with more than two named states, which legitimately calls for `Enum` — the line is the state count.
|
||||
When a field or variable represents a genuine true/false state — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same predicate as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code. This is about semantics, not member count: a domain concept that currently has exactly two named alternatives — Inbound/Outbound, Debit/Credit, Buy/Sell — is not automatically a Boolean in disguise. An `Enum` can be the clearer model there, including when it needs to implement an interface, preserve an existing contract, or leave room for a future third value. The distinction is whether the domain is genuinely a stable predicate, not how many states it currently has.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Type a true two-state field or variable as `Boolean` and branch on it directly.
|
||||
Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values.
|
||||
|
||||
See sample: `binary-choice-must-be-boolean.good.al`.
|
||||
|
||||
|
|
|
|||
|
|
@ -7,17 +7,17 @@ countries: [w1]
|
|||
application-area: [all]
|
||||
---
|
||||
|
||||
# Verify a namespace from the object's own source file, never by inference
|
||||
# Resolve a namespace from the referenced object's source or symbols, never by guessing
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. Guessing a namespace from an object's name, from an older codebase, or from general familiarity produces a `using` statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The only reliable source for an object's namespace is line one of that object's own source file.
|
||||
Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Locate the object's source file, read its `namespace` declaration on line one, and copy that exact value into the consuming file's `using` statement.
|
||||
When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention.
|
||||
|
||||
See sample: `namespace-must-be-verified-from-source.good.al`.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue