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

feat: add support for "external" stability #596

Merged
merged 5 commits into from
Jul 16, 2019

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Jul 12, 2019

Add support for a new class of stability called "imported".

This stability class is treated like "stable" for documentation purposes,
but always lead to warnings (not errors) for stability comparison purposes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Add support for a new class of stability called "imported".

This stability class is treated like "stable" for documentation purposes,
but always lead to warnings (not errors) for stability comparison purposes.
@rix0rrr rix0rrr requested a review from a team as a code owner July 12, 2019 13:46
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

I am struggling with the name... without context it’s impossible to understand why this tag is added. Why not add support for something like “@stability X” as an additional way to specify stability (think ability the current stability tags as sugar for this) and then we can had a stability type of “unknown” or “ignore”


test.deepEqual(classType.docs!.stability, spec.Stability.Imported);
test.deepEqual(method!.docs!.stability, spec.Stability.Imported);
test.done();
Copy link
Contributor

Choose a reason for hiding this comment

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

Don’t we need a test which verifies that imported can break with only warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Jul 15, 2019

We decided on @stability external

@eladb
Copy link
Contributor

eladb commented Jul 15, 2019

Discussed face to face and decided on @stability external

@eladb eladb changed the title feat: add support for "imported" stability feat: add support for "external" stability Jul 16, 2019
@RomainMuller RomainMuller force-pushed the master branch 3 times, most recently from 1315be5 to 8552b7a Compare July 16, 2019 12:54
@rix0rrr rix0rrr merged commit dd66afb into master Jul 16, 2019
@rix0rrr rix0rrr deleted the huijbers/imported-stability branch July 16, 2019 16:34
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.

None yet

3 participants