Skip to content

Clarify OneBranch localization validation under failOnValidationError - #4838

Merged
paulmedynski merged 2 commits into
mainfrom
dev/paul/aot/phase-1.01
Oct 16, 2026
Merged

paulmedynski merged 2 commits into
mainfrom
dev/paul/aot/phase-1.01

Conversation

@paulmedynski

@paulmedynski paulmedynski commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

Clarify that OneBranch localization validation participates in the shared failOnValidationError flag.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 17:38
@paulmedynski
paulmedynski requested a review from a team as a code owner October 13, 2026 17:38
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The authoritative OneBranch guidance still incorrectly states that non-official validation defaults to strict mode.

1 open finding
What changed in this PR

Defaults non-official OneBranch validation to report-only while preserving strict official validation.

Changes:

  • Adds configurable localization resource and allowlist selection.
  • Adds allowlist-isolation tests.
  • Documents validation behavior.
File Description
BUILDGUIDE.md Documents validation policy.
build-buildproj-job.yml Supplies driver localization inputs.
validate-localization-step.yml Adds explicit localization parameters.
validate-localization.ps1 Selects configurable allowlist sections.
validate-localization.Tests.ps1 Tests section isolation and errors.
sqlclient-non-official.yml Defaults validation to report-only.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread eng/pipelines/onebranch/sqlclient-non-official.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The pipeline behavior is internally consistent, but hosted validation has not completed for the current head.

1 open finding

🧠 Review effort: Balanced

@paulmedynski
paulmedynski added this pull request to stack #4839 October 13, 2026 17:45
@paulmedynski paulmedynski changed the title AOT 1.01: Default non-official validation to report-only AOT 1: Default non-official validation to report-only Oct 13, 2026
@paulmedynski paulmedynski added this to the 8.0.0-preview1 milestone Oct 13, 2026
paulmedynski added a commit that referenced this pull request Oct 13, 2026
Keep report-only validation as an explicit queue-time opt-out and align
the build guide and OneBranch guidance with the strict default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:26
@paulmedynski paulmedynski changed the title AOT 1: Default non-official validation to report-only AOT 1: Prepare localization validation for separate resource assemblies Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The non-official pipeline still defaults failOnValidationError to true, contradicting the PR’s stated purpose.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread eng/pipelines/onebranch/sqlclient-non-official.yml
Comment thread .github/instructions/onebranch-pipeline-design.instructions.md Outdated
Comment thread BUILDGUIDE.md
@paulmedynski

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
3 pipeline(s) were filtered out due to trigger conditions.

@cheenamalhotra cheenamalhotra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are these changes related to AOT again? Please don't mix terms/feature names with changes that have no links.

This should be an independent PR outside of the AOT stack.

@github-project-automation github-project-automation Bot moved this from To triage to Waiting for customer in SqlClient Board Oct 13, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation, tests, pipeline wiring, and documentation consistently preserve strict defaults while enabling explicit report-only validation.

0 open findings

3 resolved since last review

🧠 Review effort: Balanced

@paulmedynski

Copy link
Copy Markdown
Contributor Author

How are these changes related to AOT again? Please don't mix terms/feature names with changes that have no links.

This should be an independent PR outside of the AOT stack.

The sqlclient-non-official pipelines cannot be run successfully without these changes because we have new unlocalized strings on main. I'm fine to split this work out as long as sqlclient-non-official runs successfully on main, which it didn't at the time the stack was created.

Keep validation strict by default and document the report-only option. Resource-owner generalization belongs to the Abstractions localization PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 11:36
@paulmedynski
paulmedynski force-pushed the dev/paul/aot/phase-1.01 branch from 6037e56 to e5462ae Compare October 14, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The described Abstractions and per-assembly localization support is not implemented.

2 open findings

🧠 Review effort: Balanced

Comment thread .github/instructions/onebranch-pipeline-design.instructions.md Outdated
Comment thread .github/instructions/onebranch-pipeline-design.instructions.md Outdated
@paulmedynski
paulmedynski removed this pull request from stack #4839 October 14, 2026 11:39
@paulmedynski paulmedynski changed the title AOT 1: Prepare localization validation for separate resource assemblies Clarify OneBranch validation policy Oct 14, 2026
@paulmedynski
paulmedynski changed the base branch from dev/paul/aot/phase-1.0 to main October 14, 2026 11:40
@paulmedynski paulmedynski changed the title Clarify OneBranch validation policy Clarify OneBranch localization validation under failOnValidationError Oct 14, 2026
Address Copilot feedback by wrapping the validation-policy paragraph
without changing its wording or behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 12:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation consistently matches the verified pipeline behavior.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

@benrr101 benrr101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely is not P0, but alright

@paulmedynski
paulmedynski merged commit 29f76ad into main Oct 16, 2026
9 checks passed
@paulmedynski
paulmedynski deleted the dev/paul/aot/phase-1.01 branch October 16, 2026 13:15
@github-project-automation github-project-automation Bot moved this from Waiting for customer to Done in SqlClient Board Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants