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

Explaining new version naming schema #906

Merged
merged 6 commits into from
Jul 2, 2019
Merged

Explaining new version naming schema #906

merged 6 commits into from
Jul 2, 2019

Conversation

campoy
Copy link
Contributor

@campoy campoy commented Jun 27, 2019

This change is Reviewable

@campoy campoy requested a review from a team June 27, 2019 22:06
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami, @campoy, and @manishrjain)


VERSIONING.md, line 13 at r1 (raw file):

> Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

I have used and benefitted from this clear naming schema for many years, but recently I did find a small concern which drove me to propose a slightly different naming schema, specifically designed for libraries that provide encoding and decoding mechanisms which are not part of their API.

This reads like a blog post. Instead, it should focus on the reasoning behind this versioning scheme, the pros and cons and so forth.

You could include my discuss post and your medium post as reference for further clarification.

Copy link
Contributor Author

@campoy campoy left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)


VERSIONING.md, line 13 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

This reads like a blog post. Instead, it should focus on the reasoning behind this versioning scheme, the pros and cons and so forth.

You could include my discuss post and your medium post as reference for further clarification.

Agreed, I reworded the whole thing making it shorter and more to the point.

Also added links to the blog post and your comment on discuss.

Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

:lgtm: except for some minor typos/suggestions

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @ashish-goswami, @campoy, and @manishrjain)


README.md, line 78 at r2 (raw file):

- New major versions are released when the data format on disk changes in an incompatible way.
- New minor versions are released whenever the API changes but data compatibility is conserved. Note that the changes on the API could be backward-incompatible - unlike Semantic Versioning.

minor: s/conserved/maintained I think it sound a little bit better


VERSIONING.md, line 15 at r2 (raw file):

to stored

to store


VERSIONING.md, line 17 at r2 (raw file):

to stored data on disk.

## Serialization Version pecification

you mean specification?


VERSIONING.md, line 30 at r2 (raw file):

require

requires


VERSIONING.md, line 37 at r2 (raw file):

For more background on our decision to adopt Serialization Versioning, read the blog post
[Semantic Versioning, Go Modules, and Databases](https://blog.dgraph.io/post/serialization-versioning/)
and the original proposal on [this comment on Dgraph's Discuss forum](https://discuss.dgraph.io/t/go-modules-on-badger-and-dgraph/4662/7?u=mrjn).

line break at the end of the file.

Copy link
Contributor Author

@campoy campoy left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @ashish-goswami, @manishrjain, and @martinmr)


README.md, line 78 at r2 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

minor: s/conserved/maintained I think it sound a little bit better

Done


VERSIONING.md, line 13 at r1 (raw file):

Previously, campoy (Francesc Campoy) wrote…

Agreed, I reworded the whole thing making it shorter and more to the point.

Also added links to the blog post and your comment on discuss.

Done.


VERSIONING.md, line 15 at r2 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…
to stored

to store

Done.


VERSIONING.md, line 17 at r2 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

you mean specification?

Done.


VERSIONING.md, line 30 at r2 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…
require

requires

Done.


VERSIONING.md, line 37 at r2 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

line break at the end of the file.

Done.

@campoy
Copy link
Contributor Author

campoy commented Jul 1, 2019

Hey @manishrjain, could you have a look at this? Thanks!

Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)

Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ashish-goswami, @campoy, and @manishrjain)


README.md, line 18 at r4 (raw file):

the most recent branch that is data compatible
with v1.0 was just published as v1.6.0.

the latest version that is data-compatible with v1.0 is v1.6.0.


VERSIONING.md, line 25 at r4 (raw file):

- MAJOR version when you make changes that require a transformation of the dataset before it can be used again.
- MINOR version when old datasets are still readable but the API might have changed in backward-compatible or incompatible ways.

nit: backwards (just to match semver writing).


VERSIONING.md, line 26 at r4 (raw file):

- MAJOR version when you make changes that require a transformation of the dataset before it can be used again.
- MINOR version when old datasets are still readable but the API might have changed in backward-compatible or incompatible ways.
- PATCH version when you make backward-compatible bug fixes.

nit: backwards (just to match semver writing).

Copy link
Contributor Author

@campoy campoy left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @danielmai, @manishrjain, and @martinmr)


README.md, line 18 at r4 (raw file):

Previously, danielmai (Daniel Mai) wrote…
the most recent branch that is data compatible
with v1.0 was just published as v1.6.0.

the latest version that is data-compatible with v1.0 is v1.6.0.

Done.

Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @danielmai, @manishrjain, and @martinmr)

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Just ensure to keep all the lines within 100 chars.

Reviewed 1 of 1 files at r4, 2 of 2 files at r5.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ashish-goswami, @campoy, @danielmai, and @manishrjain)


README.md, line 72 at r5 (raw file):

#### Choosing a version

BadgerDB is a pretty special package from the point of view that the most important change we can make to it

100 char limit. Here and elsewhere.


VERSIONING.md, line 3 at r5 (raw file):

# Serialization Versioning: Semantic Versioning for databases

Semantic Versioning, commonly known as SemVer, is a great idea that has been very widely adopted as a way to decide how to name software versions. The whole concept is very well summarized on semver.org with the following lines:

100 char limit. Here and elsewhere.

Copy link
Contributor Author

@campoy campoy left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @ashish-goswami, @danielmai, and @manishrjain)


README.md, line 72 at r5 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 char limit. Here and elsewhere.

Done.


VERSIONING.md, line 3 at r5 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 char limit. Here and elsewhere.

Done everywhere.

@danielmai danielmai merged commit 2fa005c into master Jul 2, 2019
@danielmai danielmai deleted the comp branch July 2, 2019 00:04
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
* Explaining new Serialization Versioning schema used in Badger.

(cherry picked from commit 2fa005c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants