Skip to main content

Code Review Workflow

Every extension change goes through a structured review process before it ships. The goal is to catch bugs, performance issues, and standard deviations before they reach production — not after.

This page defines the review process, the checklists reviewers use, and the acceptance criteria for shipping code.

Code Review Workflow

Review Process​

Step 1 — Plan Before Writing​

For any non-trivial change, present 2–3 approaches with tradeoffs before writing code. Do not begin implementation until the approach is approved. This prevents wasted effort on an approach that gets rejected during review.

Step 2 — Implement​

Write the code, following the patterns and conventions in the Contribution Guidelines and Coding Standards.

Step 3 — Shipment Readiness Check​

Before requesting review, verify every item in the Shipment Readiness Checklist (below). Address any gaps before proceeding. The reviewer should not be the one discovering that documentation is missing or tests don't pass.

Step 4 — Independent Code Review​

The review must be conducted by someone who did not write the code. The reviewer evaluates the changes cold — without knowledge of the original rationale or design discussions. This is intentional. The same context that produced a decision will rationalize it on review.

The reviewer evaluates against:

  • The Shipment Readiness Checklist (completeness)
  • The Code Review Checklist (correctness and patterns)
  • The Performance Review Checklist (database access, locking, optimization)

Step 5 — Objective Verification​

Compilation and tests are the ground truth, not subjective review.

  • AL extensions: AL: Publish must succeed without errors
  • Python tools: run the script and verify the output artifact
  • Test codeunits: publish and run in the BC Test Tool (page 130401)

Step 6 — Business Logic Review​

Code review can verify syntax, patterns, and edge cases. It cannot verify that the code solves the actual business problem. A stakeholder or business owner reviews the final implementation for correctness against the original requirement before the change ships.

When to Use This Workflow​

Change TypePlan FirstShipment ChecklistCode ReviewPerformance ReviewBusiness Review
New extension or major featureRequiredRequiredRequiredRequiredRequired
New codeunit, table, or pageRequiredRequiredRequiredRequiredRequired
Modification to existing objectRecommendedRequiredRequiredRequiredRequired
Bug fix (isolated, < 20 lines)OptionalRequired (subset)RecommendedRecommendedRequired
Documentation or test plan onlyOptionalApplicable items onlyOptionalN/ARecommended

For bug fixes, the "required subset" of the shipment checklist is: documentation updated, test plan updated, tests cover the fix, tests pass, and version bumped.

Shipment Readiness Checklist​

Every extension must satisfy these requirements before it is considered ready to ship.

Documentation​

  • README.md exists in the extension folder — describes purpose, setup, configuration, and usage
  • docs/CHANGELOG.md has a new entry for this version
  • docs/CHANGE-v{version}.md exists with full detail for the current version
  • README updated if the change affects setup, configuration, or usage
  • Code is self-documenting — docstrings on public procedures, no tribal knowledge

Test Plans​

  • UAT test plan exists for the extension (generated via the Test Plan Library)
  • Test plan updated for the current change — new test cases added, changelog entry added, version bumped
  • Test plan covers the change — every user-facing behavior has at least one test case with clear expected results

Testability​

  • Business logic is in codeunits, not page/report triggers
  • Test codeunits exist for extensions with business logic
  • Tests pass in the BC Test Tool (page 130401)

Extension Configuration​

  • Telemetry configured — applicationInsightsConnectionString present in app.json
  • app.json version bumped for this change
  • ID ranges respected — all new IDs are within the extension's reserved range
  • Dependencies correct in app.json
  • NoImplicitWith enabled in app.json features

Naming & Structure​

  • File naming follows {Prefix}{ObjectName}.{ObjectType}.al
  • Folder structure follows type-based organization (Codeunit/, Table/, Page/, etc.)
  • Namespace declared for newer extensions

Code Review Checklist​

The reviewer evaluates the code against these criteria in addition to verifying the Shipment Readiness Checklist.

  • Pattern compliance — naming conventions, ID ranges, folder structure, namespace usage per the Contribution Guidelines
  • Dependency safety — no cross-extension ID reuse, correct app.json dependencies
  • Telemetry — new or modified extensions include the Application Insights connection string
  • Edge cases — null/empty handling, boundary values, concurrent access where applicable
  • Over-engineering — unnecessary abstractions, features not requested, speculative generality
  • Security — no hardcoded credentials, no unvalidated external input in AL procedures

Adversarial Review Prompts​

When reviewing code, use questions that force critical analysis rather than confirmation:

  • "What are three things wrong with this implementation?"
  • "What edge cases would break this?"
  • "If you were rewriting this from scratch, what would you do differently and why?"
  • "Does this follow the patterns in the Contribution Guidelines? List any deviations."
  • "What happens when [specific boundary condition] occurs?"
  • "Walk through the Shipment Readiness Checklist item by item. What's missing?"
  • "If this code runs under production load — 50 concurrent users, Job Queue processing every 30 seconds — where does it break?"

Do not ask "does this look good?" — it invites agreement rather than analysis.

Last updated on