Skip to content

[main] GitHub Event Requests - Batch #1740#8943

Open
AleksandricMarko wants to merge 2 commits into
mainfrom
bugs/master-GitHub-Event-Batch-1740
Open

[main] GitHub Event Requests - Batch #1740#8943
AleksandricMarko wants to merge 2 commits into
mainfrom
bugs/master-GitHub-Event-Batch-1740

Conversation

@AleksandricMarko

@AleksandricMarko AleksandricMarko commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@AleksandricMarko AleksandricMarko requested a review from a team June 30, 2026 19:13
@github-actions github-actions Bot added this to the Version 29.0 milestone Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Three integration events in CalculateRoutingLine.Codeunit.al convert existing by-value parameters to var: OnCreateLoadForwardOnBeforeEndStopLoop (TimeType, LoadFactor, CurrentWorkCenterNo), OnBeforeCreateLoadBack (TimeType, Write, CurrentWorkCenterNo), and OnBeforeCreateLoadForward (TimeType, Write, LoadFactor, CurrentWorkCenterNo).

Converting a parameter from value to var in an existing [IntegrationEvent] is a breaking API change: any extension that currently subscribes to these events must update its subscriber signature or will fail to compile. This is qualitatively different from adding new parameters at the end (also breaking, but a well-established BC platform pattern that is consistent across this PR); value-to-var conversion changes the subscriber binding contract mid-signature and is easier to miss during extension migration. Additionally, making these parameters var gives subscribers influence over routing calculation inputs (time type, load factor, work center) that the publisher reads back after firing, which expands the extension surface in a way that may not be intended.

Recommendation:

  • if this is an intentional breaking change for the next major BC release, document it explicitly in the BC breaking-changes registry and consider whether these parameters are best exposed as var (write-back) or whether read-only copies passed by value suffice; if simultaneous with the parameter additions, consider obsoletion of the old event signatures rather than in-place mutation.

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 1 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Agent 1 0 1 0 1

Totals: 0 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (13 file(s) excluded)

  • layer-disabled (knowledge) : 13 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant