Skip to content

Conversation

jmaitrehenry
Copy link
Contributor

@jmaitrehenry jmaitrehenry commented Apr 20, 2017

Proposed changes

Try to add a simplier table about semantic versioning upgrade.
Base on #2869

Related issues (optional)

Fix #2285

Try to add a simplier table about semantic versioning upgrade.
Base on docker#2869
@joaofnfernandes joaofnfernandes self-assigned this Apr 21, 2017
@mdlinville mdlinville changed the title Fix #2285 Improve semantic versioning table Apr 21, 2017
|:----|:---|:------------|----------|
| x.y.z | x+1.a.b | major upgrade | yes |
| x.y.z-1 | x+1.a.b+1 | major upgrade from an old version | no |
| x.y.z | x+1.a+1.b | major upgrade skipping minor version | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

I like it, but I think that you need to do a lot of thinking when looking at table two. I think we can make it easier by moving the Description column to be the first one.

The notation with a and b is also interesting but it introduces a lot of complexity. Since the first minor and patch versions are always 0, you can do

From To Description Supported
x.y.z x+1.0.0 major upgrade yes
x.y.z x+1.y+1.z major upgrade, skipping minor version no

The case for "major upgrade from an old version" is a bit special and few users are running that configuration, so we can leave it out.
I think this is a good compromise between accuracy and simplicity. #SimpleDocsFTW

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I already remove some line because when I read it, it was complicated for me too.
Thanks for your eyes!

@jmaitrehenry
Copy link
Contributor Author

@joaofnfernandes ready for a new review!

Copy link

@mdlinville mdlinville left a comment

Choose a reason for hiding this comment

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

This is really picky, but since the table isn't too wide, can you line up the pipes so it is easier to see the table boundaries in the raw Markdown? Just pad it with spaces.

| skip major version | x.*.* | x+2.*.* | no |
| major downgrade | x.*.* | x-1.*.* | no |

In the next table, y and z are the latest minor or patch version and a,b the
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're not using a and b you can remove this sentence, and make this a single table again. After that and padding the table, we're ready to ship.

Pro tip: if you're using Atom, there's this markdown-table-formatter that automatically fixes tables when you save.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check if we have the same kind of plugin for vscode, thanks for the protip!

@jmaitrehenry
Copy link
Contributor Author

@mstanleyjones and @joaofnfernandes ready for a new review :)

Copy link
Contributor

@joaofnfernandes joaofnfernandes left a comment

Choose a reason for hiding this comment

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

Looks good, ready to ship 🚀

@joaofnfernandes
Copy link
Contributor

I don't see anything here that would make the build break, and was seeing some errors with the Netlify APIs. Kicking it off again to see if the works this time.

@mdlinville mdlinville merged commit f3973ab into docker:master Apr 21, 2017
@jmaitrehenry jmaitrehenry deleted the patch-1 branch April 23, 2017 00:22
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.

4 participants