-
Notifications
You must be signed in to change notification settings - Fork 689
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
Make notice about leading zeros clearer #282
base: master
Are you sure you want to change the base?
Conversation
It was very unclear to me that "must not contain leading zeroes" is referring to "non-negative integers" rather than to "A normal version number". First (correct) interpretation would disallow 1.01.1, second (incorrect) would disallow 0.1.0. I tried to make it more explicit here.
Never had a problem with that, nor heard of anyone who did. Edit: Though, I can see how the comma makes it ambiguous. |
Well, perhaps it's just me |
Honestly, I'm not really sure that the change clarifies anything. |
Besides the contents of this PR, I also support the point of improving the meaning of |
After re-reading, I agree that this change is a bit easier to understand. Disregard my last. |
Of course, I don't think my current edit is ideal, and I am pretty sure someone could suggest further improvements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one possible alternate suggestion for consideration.
non-negative integers, and MUST NOT contain leading zeroes. X is the | ||
major version, Y is the minor version, and Z is the patch version. | ||
non-negative integers. Elements (X, Y and Z) MUST NOT contain leading zeroes. | ||
X is the major version, Y is the minor version, and Z is the patch version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about just changing it from the "...MUST NOT contain leading zeroes..." in the original as:
...except for a single character ('0') place holder, MUST NOT contain leading zeroes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is implied. A single 0
number does not have leading zeroes since the single zero digit is required here. This is universally understood in all other occasions I've seen this being discussed in one way or another.
I also dislike the phrasing of '0' being a "place holder", since it's clearly not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KISS:
A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers.
X, Y, and Z MUST NOT contain leading zeroes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-breaking incremental change, looks good to me.
Closing & re-opening to trigger CI |
It was very unclear to me that "must not contain leading zeroes" is referring to "non-negative integers" rather than to "A normal version number".
First (correct) interpretation would disallow 1.01.1, second (incorrect) would disallow 0.1.0.
I tried to make it more explicit here.