Fail CI build if there are any non-baselined API changes#38089
Fail CI build if there are any non-baselined API changes#38089AndriySvyryd merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates CI pipelines to use the repo’s build.cmd/build.sh entrypoints so that API baseline generation/delta checking runs during CI and fails the build when API changes aren’t baselined.
Changes:
- Switched public pipeline build steps from
eng/common/cibuild.*to./build.*so CI runstools/MakeApiBaselines.ps1. - Switched the internal-tests Windows build step from
eng\common\cibuild.cmdtobuild.cmdto align with the new API baseline enforcement path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| azure-pipelines-public.yml | Uses repo build.cmd/build.sh in public CI builds, enabling API baseline enforcement. |
| azure-pipelines-internal-tests.yml | Updates the internal Windows build to use build.cmd, which runs API baseline checks. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
eng/Tools/ApiChief/Model/ApiType.cs:30
- Changing
ApiTypetopublicmakes its shape part of ApiChief's public surface area, including the public mutable fields (Methods,Fields,Properties). If these types are only being made public to support the new test project, consider keeping theminternaland addingInternalsVisibleTofor the test assembly (or exposing a narrower public API) to avoid locking in these implementation details.
public sealed class ApiType : IEquatable<ApiType>
{
[JsonIgnore]
public string FullTypeName { get; set; } = string.Empty;
[JsonPropertyOrder(0)]
public string Type { get; set; } = string.Empty;
[JsonPropertyOrder(1)]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public ApiStage Stage { get; set; }
[JsonPropertyOrder(2)]
public ISet<ApiMember>? Methods;
[JsonPropertyOrder(3)]
public ISet<ApiMember>? Fields;
[JsonPropertyOrder(4)]
public ISet<ApiMember>? Properties;
roji
left a comment
There was a problem hiding this comment.
Added the new EFCore.ApiBaseline.Tests project that will fail on CI if any baseline is outdated and silently update it when run locally.
I'd prefer it if the project failed locally as well, and to just run an explicit command to update it.
BTW consider adding info on how all this works in the repo instructions file, for agents (and for Shay...).
Why? How often do you introduce or change public API unintentionally? You will still clearly see the baseline changes after the test is run. Adding an extra step seems would just slow you down (and add frustration) without any benefit for the vast majority of cases. |
It happens; after all, one of the points of having public API baselines (and a setup to fail if the baseline isn't aligned to the source) is precisely to protect against unintentional public API changes. Public API changes feel important enough that they should involve an intentional gesture. I also kinda dislike the same thing doing one thing in CI and a different thing locally (e.g. more difficult to debug/understand when things go wrong). No super strong feelings though, if you really care I'm OK with your way too. |
API review baseline changes for
|
EFCore.ApiBaseline.Testsproject that will fail on CI if any baseline is outdated and silently update it when run locally..baseline.jsonfiles forEFCore.Cosmos,EFCore.Relational,EFCore.SqlServer, andEFCoreto reflect new, changed, or removed API members and types.ApiChief) to: