-
Notifications
You must be signed in to change notification settings - Fork 720
Adding workflow to automatically compare public API surface against previous release #7369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Can we exclude the dashboard |
|
We're going to need to exclude a set of projects. |
|
Sure, apart from dashboard, which others come to mind? If not many we can just skip in the workflow itself, if many then we can instead fine-tune the targets so they only run in case we ship them as packages |
|
src/Tools/ConfigurationSchemaGenerator |
aabb2d4 to
792ee13
Compare
Ok, done. I've changed it such that we only do this for projects that have 'IsPackable' set to true, which will automatically exclude things like Dashboard and internal tools. I've also updated the 9.0 api surface to only add the relevant files here. I tested this workflow on my fork, and PRs seem to be generated correctly, so I think this is ready for feedback to go in. |
|
Here is what the diff between 9.0 and 9.1 looks like in my fork: joperezr#4 |
|
@eerhardt I'm merging now in order to test that the auto-PRs get created correctly on the main fork, but if you (or anyone else) has any other feedback I'll gladly address it in a follow-up. |
This is no longer necessary after dotnet#7369
This is no longer necessary after #7369
| --> | ||
| <NoWarn>$(NoWarn);CS1570;CS1591</NoWarn> | ||
| <!--<DefineConstants>$(DefineConstants);LAUNCH_DEBUGGER</DefineConstants>--> | ||
| <RollForward>Major</RollForward> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joperezr - what was this needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of building each project and running the task to generate the ref source, the schema generator was being invoked (not sure if by design or that was not expected). Given the only dotnet we were installing is the one in global.json (which was 9.0.x) and this tool was targeting net8, the build was failing saying that in order for this tool to run we'd need to install net8 shared fx. By adding rollforward this tool is now able to run with a newer sdk.
Independently on whether or not it was expected for this to run as part of the build, I thought this was a change worth doing anyway as we want people who only have the 9.0 sdk without the net8 shared fx to be able to build and generate the schema.
Description
This PR adds a workflow that runs once a day (or can also be triggered on-demand) which will re-generate the API surface files with latest code in main branch, and will send (or update if one is open already) a PR to show what has changed since last release. Once a new release ships, this PR will be merged, so that the new PR that gets created the next day will show now the diff against the recently shipped released.
This PR is also adding all the API surface as it was in our 9.0.0 release, so that the PR that gets created once we merge this, will show the new surface to be introduced in 9.1. The purpose of the generated PR is to use it to review the new API, and add comments and make corrections in between releases so that we are happy with the API to be released in the next version.
PR is best reviewed commit by commit:
cc: @davidfowl @ericstj @DamianEdwards @danmoseley
Checklist
<remarks />and<code />elements on your triple slash comments?breaking-changetemplate):doc-ideatemplate):