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

Remove upper case constants #92

Merged
merged 6 commits into from
Apr 16, 2018

Conversation

lrhn
Copy link
Contributor

@lrhn lrhn commented Apr 11, 2018

No description provided.

Copy link
Contributor

@jakobr-google jakobr-google left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,3 +1,7 @@
## 0.7.2+1

- Updated SDK version to 2.0.0-dev.17.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add:

  • Switched to Dart 2 constant names

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an invisible change, so I don't see that as giving the reader of the changelog any useful information.
A developer can see it in the commit message instead.

@@ -1,10 +1,10 @@
name: protobuf
version: 0.7.2
version: 0.7.2+1
Copy link
Contributor

Choose a reason for hiding this comment

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

0.7.3. I've never really understood the +NNN silliness. They're for publishing different builds of the same version, but this is a new version. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way we have traditionally handled versions below 1.0.0 in the Dart project is to treat it as a full three-digit semantic version system shifted down by one, so a patch increment over 0.7.2 is 0.7.2+1.
It's not official semantic versioning, just Dart habit. I have no problem going to 0.7.3 or 0.8.0 if you want that instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're putting far too much thought into the version numbers. :)

I'll take this as is, and update the Travis config to disable running on stable.

@jakobr-google
Copy link
Contributor

Travis is unhappy. We'll need to disable running on stable, and move the dartfmt task to dev.

@jakobr-google jakobr-google merged commit d2c82bb into google:master Apr 16, 2018
sigurdm pushed a commit to sigurdm/protobuf that referenced this pull request Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants