Skip to content
This repository has been archived by the owner on Mar 8, 2021. It is now read-only.

Fix member removal check #7

Merged
merged 1 commit into from
Aug 24, 2017
Merged

Fix member removal check #7

merged 1 commit into from
Aug 24, 2017

Conversation

jonpryor
Copy link
Member

Context: dotnet/android#771 (comment)

Checking for >Removed is insufficient for checking for API
compatibility breakage, as sometimes mono-api-html will emit
>Modified.

For example, xamarin-android PR #771 changes the type of the
Android.App.NotificationChannel.LockscreenVisibility property from
int to NotificationVisibility. This was expected to result in a
build failure, due to API breakage.

It didn't.

Update the member removal check to instead check for the existence of
the data-is-breaking tag. This will hopefully improve compatibility
checks in the future.

Context: dotnet/android#771 (comment)

Checking for `>Removed` is insufficient for checking for API
compatibility breakage, as sometimes `mono-api-html` will emit
`>Modified`.

For example, [xamarin-android PR #771][xa771] changes the type of the
`Android.App.NotificationChannel.LockscreenVisibility` property from
`int` to `NotificationVisibility`. This was expected to result in a
build failure, due to API breakage.

[xa771]: dotnet/android#771 (comment)

It didn't.

Update the member removal check to instead check for the existence of
the `data-is-breaking` tag. This will hopefully improve compatibility
checks in the future.
@atsushieno
Copy link
Contributor

Is data-is-breaking really the only message the tool reports? I think there were some more. Thus I'm quite unsure if replacing the checked tag with it is a good idea...

@atsushieno
Copy link
Contributor

I just read the mono-api-html sources and I think it can be regarded as identifying ABI breakage.

@atsushieno atsushieno merged commit 4e0acfb into master Aug 24, 2017
@jonpryor jonpryor deleted the jonp-fix-removal-check branch August 25, 2017 13:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants