Skip to content

[APIDiff] Support forwarding breakage allowlists to the api digester #3547

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

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

owenv
Copy link
Contributor

@owenv owenv commented Jun 12, 2021

Add a --breakage-allowlist-path flag to allow ignoring specified breaking changes when running experimental-api-diff

Motivation:

Sometimes, it's beneficial to be able to ignore certain API breaking changes when perfect API compatibility isn't necessary.

Modifications:

  • Adds a new --breakage-allowlist-path flag which is forwarded directly to the api digester's diagnose-sdk action. If the flag isn't specified, the tool will also check to see if api-breakage-allowlist.txt exists at the package root and use that.

Result:

Users can choose to ignore certain API-breaking changes when running comparisons

@owenv
Copy link
Contributor Author

owenv commented Jun 12, 2021

@swift-ci smoke test

The path to a text file containing breaking changes which should be ignored by the API comparison. \
Each ignored breaking change in the file should appear on its own line and contain the exact message \
to be ignored (e.g. 'API breakage: func foo() has been removed'). If this option is not present, \
a file named 'api-breakage-allowlist.txt' at the package root will be used if it exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

The default behavior doesn't seem intuitive for this input option and it would be cleaner to just require a specific path to be passed in by a user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good to me!

@owenv owenv force-pushed the breakage-allowlist branch from b27e14a to e6b7061 Compare June 18, 2021 04:31
@owenv
Copy link
Contributor Author

owenv commented Jun 18, 2021

@swift-ci smoke test

@owenv
Copy link
Contributor Author

owenv commented Jun 20, 2021

@swift-ci smoke test

@owenv
Copy link
Contributor Author

owenv commented Jun 26, 2021

@swift-ci please smoke test macOS

@tomerd
Copy link
Contributor

tomerd commented Jun 29, 2021

@owenv @elsh is this ready to be merged?

@owenv
Copy link
Contributor Author

owenv commented Jun 29, 2021

yeah, I think this should be ok to merge now

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.

3 participants