mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
f19f0618fb
* Add events knowledge domain and review leaf skill Add a new `events` knowledge domain covering AL events & subscribers, wired into the AL review pipeline. - 3 atomic articles (+ .good.al/.bad.al samples) under microsoft/knowledge/events/: the IsHandled override pattern, thin OnBefore/OnAfter integration-event publishers, and static vs manual subscribers. - New leaf skill microsoft/skills/review/al-events-review.md sourcing the events domain. - Wired into microsoft/skills/review/al-code-review.md (sub-skills + Source + description) and README.md (leaf-skill count + domain list). AL event syntax verified against Microsoft Learn. Samples are demonstration-only (not compiled by CI). Additive change; no contract change. Part of #34. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add 12 general AL event-design articles to events domain Add 12 atomic knowledge articles under microsoft/knowledge/events covering general AL event-design best practices: IsHandled initialization and OnAfter preservation, appending new event parameters, position-based event naming, reusing/extending events, avoiding per-iteration publishing, Temp-prefixing temporary record parameters, unabbreviated parameter names, preferring the this keyword over IncludeSender, avoiding loosely typed parameters, not mutating existing event contracts, and not bypassing critical operations with IsHandled. Each article ships a .good.al and .bad.al demonstration sample (object IDs 50240-50296; not compiled by CI). Extend the al-events-review leaf Worklist with one targeted check per new rule. Additive only; no contract or wiring change (events leaf already wired). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refine events articles after review feedback Correct wording in five events articles to reflect that AL event subscribers bind by parameter name, not position: - add-new-event-parameters-at-the-end: drop the inaccurate claim that appending a parameter forces subscribers to be updated or causes wrong values; keep the append-at-end best practice. - do-not-add-ishandled-to-an-existing-event: reframe from "breaking change" to the semantic/purpose shift that leaves existing subscribers pointless; rename the breaking-change keyword to semantic-change. - name-events-by-publisher-position: extend the good sample with position-named publishers raised from table and report trigger contexts. - initialize-ishandled-to-false-before-publishing: scope the detection and best practice to events that actually carry a var IsHandled, so an OnBefore with no IsHandled is not flagged. - do-not-bypass-critical-operations-with-ishandled: add a litmus-test definition of a critical operation (code that cannot stand as an independent, self-contained unit). Knowledge-only; no contract or wiring change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Soften Anti Pattern wording in add-new-event-parameters article Remove the last name-vs-position misconception from the Anti Pattern so it is consistent with the corrected Description: mid-list insertion is framed as noisy and harder to review rather than as forcing subscriber re-mapping. Detection sentence unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add events domain reviewers to CODEOWNERS Add @AleksandricMarko and @pchriste-microsoft-com as required reviewers for the events knowledge domain, matching the existing per-domain expert ownership convention. Inserted in alphabetical order ahead of the performance line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
26 lines
1.6 KiB
Markdown
26 lines
1.6 KiB
Markdown
---
|
|
bc-version: [all]
|
|
domain: events
|
|
keywords: [parameter-naming, readability, conventions, event-parameters, no-abbreviations, integration-event, clarity]
|
|
technologies: [al]
|
|
countries: [w1]
|
|
application-area: [all]
|
|
---
|
|
|
|
# Name event parameters without abbreviations
|
|
|
|
## Description
|
|
|
|
Event parameter names are part of the public contract a subscriber codes against, so they must be self-explanatory. Record parameters take the full table name with the spaces removed — `SalesHeader` for `"Sales Header"`, not `SalesHdr` or `SH`. Simple parameters get a descriptive, spelled-out name — `DocumentNo`, not `DocNo`; `Amount`, not `Amt`. Abbreviated names force every subscriber author to guess intent and tend to be inconsistent across a codebase, where the same concept appears under several contractions. The cost of a clear name is paid once at the publisher; the cost of a cryptic one is paid by every subscriber that has to decode it.
|
|
|
|
## Best Practice
|
|
|
|
Use full, unabbreviated names: `(SalesHeader: Record "Sales Header"; DocumentNo: Code[20]; Amount: Decimal)`. Record parameters mirror the table name without spaces, and value parameters read as whole words so the contract is unambiguous.
|
|
|
|
See sample: `name-event-parameters-without-abbreviations.good.al`.
|
|
|
|
## Anti Pattern
|
|
|
|
Abbreviated parameter names (`SalesHdr`, `DocNo`, `Amt`) that obscure meaning and vary across publishers, so subscribers must guess what each one holds. Detection: event parameters whose names are truncated forms of the table name or contracted words rather than the full term.
|
|
|
|
See sample: `name-event-parameters-without-abbreviations.bad.al`.
|