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

[AIP-132] Remove requirement to document ordering. #512

Merged
merged 3 commits into from May 27, 2020
Merged

Conversation

lukesneeringer
Copy link
Contributor

I originally planned to add an example as the person who posted #506 suggested, but it turns out that literally nobody documents this. As best as I can tell, this has never been a problem for anyone, and therefore I believe we should just remove the requirement.

Fixes #506.

Luke Sneeringer added 2 commits May 19, 2020 13:17
I originally planned to add an example as the person who posted #506
suggested, but it turns out that _literally nobody_ documents this.
As best as I can tell, this has never been a problem for anyone, and
therefore I believe we should just remove the requirement.

Fixes #506.
@lukesneeringer lukesneeringer requested a review from a team May 19, 2020 20:19
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 19, 2020
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Any reason for removing the requirement entirely rather than just softening it to "should"? It feels like it's still a good thing to do where possible.

@lukesneeringer
Copy link
Contributor Author

I think this falls into the category of "not actually needed". I can not find any API that actually documents this (I looked), and it seems to cause no user consternation. No API reviewer checks for this. We have no good enforcement mechanism since there is no machine-readable way to check, either.

I prefer not have "dead law" -- if it is not being done, and it is not causing pain, and I do not intend to put any effort toward changing it, then I think it is better to be silent / permissive.

Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Okay, works for me.

@lukesneeringer lukesneeringer merged commit 1989117 into master May 27, 2020
@lukesneeringer lukesneeringer deleted the issue-506 branch May 27, 2020 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AIP-132 does not show how to specify default order
3 participants