Should we use Semantic Versioning in release numbers #3709

Closed
romani opened this Issue Jan 10, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Jan 10, 2017

http://semver.org
format must be "X.Y.Z", some tools warn about this - https://www.versioneye.com/java/com.puppycrawl.tools:checkstyle/7.4

so, do we need to switch to that version format ?

Benefits:

  • follow the standard user can clearly see backward-incomptible fixes

Inconveniences and nuances:

  • does not work well with time-based releases, we can not make all "planned" updates in one iteration, so we will trigger increment of major version too often to my mind. I even think to use continuous releases, but it also have problem with semantic versions - 1, 2
  • extra 0 at the end of release
  • we do not completely follow "backwards-compatible" in common sense, we introduce NOT backwards-compatible changes in minor release. We do this as too much in checkstyle is exposed to user.
  • maven release plugin will automatically update version to X.Y.Z+1. So for 7.4.0 it will do 7.4.1 , so we will need to be caution during release.

Why we use time-based releases ?

Maintainers team is tiny. Non of maintainers focused on project full hours and do progress base on their free time. So big updates are impossible to accomplish in one release. There are many occasional contributors that provide fixes to bugs. All this contributors need is to pass acceptance of fix and start to use result as soon as possible (the only incentive to contribute). So delay in release will discourage contributors - not acceptable. I selected 1 month period , only because release process is time consuming and I do not want to spend my time on releases only. This period is easy to remember for me.
In short: why we are small team that do maintenance during free time (unpredictable time) so we should appreciate contribution from aside, so release often, tom time-based.

What is api changes for Checkstyle ?

Checkstyle has api package for projects that want to embed it (plugins, ....). Checkstyle exposing configuration file there all properties and modules are mentioned. We have huge problems in API classes and maintaining badly designed modules and their properties.
Change in API affect other projects that use us, checkstyle in modules affect all our users.
So all modules and module's properties are critical point in backward compatibility.
During last 3 years, we did a brake in 17 releases. Some modules are still so badly designed so we will continue that brake way for sure (still too much to break). API is slightly changed during this time.

What does checkstyle numbers mean now ?

it is partly semantic.
Major version is major milestone - so all "planned" fixes will eventually appear in it. I usually bind it to API classes major refactoring, big features introduction, runtime jdk change.
Minor version is minor milestones and minor breaking compatibilities - I consider changes on breaking changes in modules as minor break. I bump minor if new functionality is appear.
Patch version - is purely semantic patch version, completely compatible, no new functionality.

@jochenvdv

This comment has been minimized.

Show comment
Hide comment
@jochenvdv

jochenvdv Jan 10, 2017

Contributor

I think this is a good idea, it makes it clear when releases break backwards compatibility.

we do not completely follow "backwards-compatible" in common sense, we introduce NOT backwards-compatible changes in minor release.

Indeed we need to make sure we adhere to the standard strictly and not introduce backwards incompatible changes in minor releases, only in major ones, else the benefit of semantic versioning is lost. According to the SemVer standard this does not however apply to non-public APIs I believe, these can be changed every minor (or even patch) release and should be marked as such.

Contributor

jochenvdv commented Jan 10, 2017

I think this is a good idea, it makes it clear when releases break backwards compatibility.

we do not completely follow "backwards-compatible" in common sense, we introduce NOT backwards-compatible changes in minor release.

Indeed we need to make sure we adhere to the standard strictly and not introduce backwards incompatible changes in minor releases, only in major ones, else the benefit of semantic versioning is lost. According to the SemVer standard this does not however apply to non-public APIs I believe, these can be changed every minor (or even patch) release and should be marked as such.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 10, 2017

Member

we do not completely follow "backwards-compatible" in common sense, we introduce NOT backwards-compatible changes in minor release.

If we are going to follow the format then it seems to me we need to follow what the numbers mean, otherwise I would rather stick to our current model, even if it isn't standard.

So the question becomes: Are we going to change how we release major/minor/patch changes and how are we going to do that?

We have a number of checkstyle 8 breaking changes, I assume all of them have to be released in 8.0.0 before we can go to 8.X.Y, otherwise the next version should become 9.0.0, right? This would require to have people actually work on them, instead users picking and choosing fixes they would like to work on. That in turn requires us to better plan our issue schedule, which would require us to go back and remark all current issues with more depth. If we don't have many users working on issues, especially ones they aren't familiar with, release schedule might not be able to maintain one month release.

extra 0 at the end of release

If we truly follow the versioning, we won't always have a 0 on the end.

Member

rnveach commented Jan 10, 2017

we do not completely follow "backwards-compatible" in common sense, we introduce NOT backwards-compatible changes in minor release.

If we are going to follow the format then it seems to me we need to follow what the numbers mean, otherwise I would rather stick to our current model, even if it isn't standard.

So the question becomes: Are we going to change how we release major/minor/patch changes and how are we going to do that?

We have a number of checkstyle 8 breaking changes, I assume all of them have to be released in 8.0.0 before we can go to 8.X.Y, otherwise the next version should become 9.0.0, right? This would require to have people actually work on them, instead users picking and choosing fixes they would like to work on. That in turn requires us to better plan our issue schedule, which would require us to go back and remark all current issues with more depth. If we don't have many users working on issues, especially ones they aren't familiar with, release schedule might not be able to maintain one month release.

extra 0 at the end of release

If we truly follow the versioning, we won't always have a 0 on the end.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 12, 2017

Member

@jochenvdv ,

Indeed we need to make sure we adhere to the standard strictly and not introduce backwards incompatible changes in minor releases

I am afraid to run major version to quickly. May be after stabilizing API and problematic design of existing modules we could switch to that model.
I extended issue description to have more details.


@rnveach ,
I extended issue description a bit , please read.

We have a number of checkstyle 8 breaking changes, I assume all of them have to be released in 8.0.0 before we can go to 8.X.Y, otherwise the next version should become 9.0.0, right?

We will never release all that changes in one release. That is why I am kind of against blind following of semantic version model - it does not work well for time-based releases with non completely focused team for whole day.

This would require to have people actually work on them, instead users picking and choosing fixes they would like to work on.

there are almost no maintainers, so forcing people does not work. As we can not afford planned work, we use "controlled bazar" and do some progress.

That in turn requires us to better plan our issue schedule, which would require us to go back and remark all current issues with more depth.

I never succeeded in this, I doubt it is possible in our model of work. Time of maintainers is contributors is too unpredictable. Issues that bother them first should be done first to give them intensive to contribute. Maintainers could use planned focus, but we need more members to make it work.

If we don't have many users working on issues, especially ones they aren't familiar with, release schedule might not be able to maintain one month release.

yes, that is why time based release period (1 month) was selected by me a while ago.

If we truly follow the versioning, we won't always have a 0 on the end.

we have to, as by standard there MUST be 3 digits.
So we will have trailing zero in most of releases as in each release we do minor break or introduce smth new. You can see this yourself - http://checkstyle.sourceforge.net/releasenotes.html .
Another example is sevntu extension, it has to follow that 3-digit version format as it is required by Eclipse (eslipse-cs extension is failing to build if format is not 3-digit, might be some demand of OSGI or ....).

Member

romani commented Jan 12, 2017

@jochenvdv ,

Indeed we need to make sure we adhere to the standard strictly and not introduce backwards incompatible changes in minor releases

I am afraid to run major version to quickly. May be after stabilizing API and problematic design of existing modules we could switch to that model.
I extended issue description to have more details.


@rnveach ,
I extended issue description a bit , please read.

We have a number of checkstyle 8 breaking changes, I assume all of them have to be released in 8.0.0 before we can go to 8.X.Y, otherwise the next version should become 9.0.0, right?

We will never release all that changes in one release. That is why I am kind of against blind following of semantic version model - it does not work well for time-based releases with non completely focused team for whole day.

This would require to have people actually work on them, instead users picking and choosing fixes they would like to work on.

there are almost no maintainers, so forcing people does not work. As we can not afford planned work, we use "controlled bazar" and do some progress.

That in turn requires us to better plan our issue schedule, which would require us to go back and remark all current issues with more depth.

I never succeeded in this, I doubt it is possible in our model of work. Time of maintainers is contributors is too unpredictable. Issues that bother them first should be done first to give them intensive to contribute. Maintainers could use planned focus, but we need more members to make it work.

If we don't have many users working on issues, especially ones they aren't familiar with, release schedule might not be able to maintain one month release.

yes, that is why time based release period (1 month) was selected by me a while ago.

If we truly follow the versioning, we won't always have a 0 on the end.

we have to, as by standard there MUST be 3 digits.
So we will have trailing zero in most of releases as in each release we do minor break or introduce smth new. You can see this yourself - http://checkstyle.sourceforge.net/releasenotes.html .
Another example is sevntu extension, it has to follow that 3-digit version format as it is required by Eclipse (eslipse-cs extension is failing to build if format is not 3-digit, might be some demand of OSGI or ....).

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 12, 2017

Member

from semantic description:

MAJOR version when you make incompatible API changes

What is "incompatible API changes" for you ?
is it incompatible changes in API classes ? (affect other projects, where checkstyle is embeded)
is it incompatible changes in configuration ? (affects all config in whole world)

incompatible mean

  • removal of class from api ?
  • removal of module ?
  • removal of property in module ?
  • serious change in module logic ?
  • change for module behavior ?
Member

romani commented Jan 12, 2017

from semantic description:

MAJOR version when you make incompatible API changes

What is "incompatible API changes" for you ?
is it incompatible changes in API classes ? (affect other projects, where checkstyle is embeded)
is it incompatible changes in configuration ? (affects all config in whole world)

incompatible mean

  • removal of class from api ?
  • removal of module ?
  • removal of property in module ?
  • serious change in module logic ?
  • change for module behavior ?
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 12, 2017

Member

If we truly follow the versioning, we won't always have a 0 on the end.

we have to, as by standard there MUST be 3 digits.

We must have 3 digits, yes, but it won't always be a 0. See release 7.1.2. I was only referring to the fact its not always 0.

What is "incompatible API changes" for you ?

Any public method signature change. Modifier change (public -> private, non-static to static), name change, parameter type change, number of parameter change, etc. Anything that would break external code that made reference to method without making changes in source.
I didn't think of class level, but yes, class removal is an API change. New method isn't API change unless its abstract.
Logic change is not API change unless we violate original javadoc, and it must be an API method/class (public) that is being violated. This area is probably too general for me to expand on more.
Simple example: We change return in method from -1 to mean nothing found to -2, and now -1 means something special/new.

Obviously most of this is too strict for us. We are constantly finding flaws in the logic of our own code that is years old. That is what most of checkstyle 8 labels are.

May be after stabilizing API and problematic design of existing modules we could switch to that model.

I agree. Stick to current model and switch to 3 digits and semver when we can. Doing it now would probably confuse people when we break stuff in-between major releases.

It is also not clear what is really API and what is not in checkstyle. Check classes are in a sense API (user calls them directly in config) but they don't reside in API folder. Checker is not in API folder but 3rd party plugins call it directly for their use as their way to use checkstyle outside the CLI. If we make changes to Checker we break those plugins, so it is an API now.
We should clearly define what classes are API, and only support breaking compatibility in them. If, for example, Checker shouldn't be API, we would need to give them a way to instantiate the RootModule in an API way and not support them accessing Checker directly.

Member

rnveach commented Jan 12, 2017

If we truly follow the versioning, we won't always have a 0 on the end.

we have to, as by standard there MUST be 3 digits.

We must have 3 digits, yes, but it won't always be a 0. See release 7.1.2. I was only referring to the fact its not always 0.

What is "incompatible API changes" for you ?

Any public method signature change. Modifier change (public -> private, non-static to static), name change, parameter type change, number of parameter change, etc. Anything that would break external code that made reference to method without making changes in source.
I didn't think of class level, but yes, class removal is an API change. New method isn't API change unless its abstract.
Logic change is not API change unless we violate original javadoc, and it must be an API method/class (public) that is being violated. This area is probably too general for me to expand on more.
Simple example: We change return in method from -1 to mean nothing found to -2, and now -1 means something special/new.

Obviously most of this is too strict for us. We are constantly finding flaws in the logic of our own code that is years old. That is what most of checkstyle 8 labels are.

May be after stabilizing API and problematic design of existing modules we could switch to that model.

I agree. Stick to current model and switch to 3 digits and semver when we can. Doing it now would probably confuse people when we break stuff in-between major releases.

It is also not clear what is really API and what is not in checkstyle. Check classes are in a sense API (user calls them directly in config) but they don't reside in API folder. Checker is not in API folder but 3rd party plugins call it directly for their use as their way to use checkstyle outside the CLI. If we make changes to Checker we break those plugins, so it is an API now.
We should clearly define what classes are API, and only support breaking compatibility in them. If, for example, Checker shouldn't be API, we would need to give them a way to instantiate the RootModule in an API way and not support them accessing Checker directly.

@jochenvdv

This comment has been minimized.

Show comment
Hide comment
@jochenvdv

jochenvdv Jan 12, 2017

Contributor

I agree as well, we should wait until the API is stable. Having a new major version released every couple weeks is ridiculous.

Contributor

jochenvdv commented Jan 12, 2017

I agree as well, we should wait until the API is stable. Having a new major version released every couple weeks is ridiculous.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 12, 2017

Member

@jochenvdv , @rnveach ,
So I presume it is ok to stay with non-semver format (2-digit or 3-digit) to even not mimic that we are follow semantic versioning.

Member

romani commented Jan 12, 2017

@jochenvdv , @rnveach ,
So I presume it is ok to stay with non-semver format (2-digit or 3-digit) to even not mimic that we are follow semantic versioning.

@jochenvdv

This comment has been minimized.

Show comment
Hide comment
@jochenvdv

jochenvdv Jan 12, 2017

Contributor

Yes, we should stick with the current 2-digit format for now.

Contributor

jochenvdv commented Jan 12, 2017

Yes, we should stick with the current 2-digit format for now.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 12, 2017

Member

@romani Yes, that is my stance.

Member

rnveach commented Jan 12, 2017

@romani Yes, that is my stance.

@romani romani closed this Jan 12, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 10, 2018

Member

for now we use Romantic/Sentimental versioning http://dafoster.net/articles/2015/03/14/semantic-versioning-vs-romantic-versioning/

Ones we finish removing all implementations from our API package , we can switch to semantic versioning.

Member

romani commented May 10, 2018

for now we use Romantic/Sentimental versioning http://dafoster.net/articles/2015/03/14/semantic-versioning-vs-romantic-versioning/

Ones we finish removing all implementations from our API package , we can switch to semantic versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment