Skip to content
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

Update the api check. #1670

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Update the api check. #1670

merged 1 commit into from
Jul 8, 2024

Conversation

thomasvl
Copy link
Collaborator

@thomasvl thomasvl commented Jul 8, 2024

  • With main now being used for releases, just need to check against the last release.
  • Remove all the known breaks issues as we've done a release and thus reset the list.

- With `main` now being used for releases, just need to check against the last release.
- Remove all the known breaks issues as we've done a release and thus reset the list.
@thomasvl thomasvl requested a review from allevato July 8, 2024 14:43
@thomasvl
Copy link
Collaborator Author

thomasvl commented Jul 8, 2024

@gjcairo @FranzBusch @tbkka fyi.

@thomasvl thomasvl merged commit b4d353b into apple:main Jul 8, 2024
10 checks passed
Copy link
Member

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

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

I would recommend to do the inverse and keep the breaking change against main and remove the one against the latest release. We do this in our other repos. This is really useful because sometimes the tool reports ABI but not API breaks which we force-merge over. If we compare against the release we have to force merge until we cut another release.

@thomasvl
Copy link
Collaborator Author

thomasvl commented Jul 8, 2024

I would recommend to do the inverse and keep the breaking change against main and remove the one against the latest release. We do this in our other repos. This is really useful because sometimes the tool reports ABI but not API breaks which we force-merge over. If we compare against the release we have to force merge until we cut another release.

Sorry, already hit submit. Will follow up.

So you don't bother with a known api breaks file then?

@thomasvl thomasvl deleted the update_api_check branch July 8, 2024 14:49
@FranzBusch
Copy link
Member

So you don't bother with a known api breaks file then?

Correct. We make the decision once in a PR where we have to merge over it and then consider it baseline again.

@thomasvl
Copy link
Collaborator Author

thomasvl commented Jul 8, 2024

So you don't bother with a known api breaks file then?

Correct. We make the decision once in a PR where we have to merge over it and then consider it baseline again.

Someone on the Apple side will have to do any configuration need to allow the force merge, I'm not sure if something needs to be tweaked in the project configuration on GitHub to allow that.

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.

4 participants