multi-module-maven-release-plugin icon indicating copy to clipboard operation
multi-module-maven-release-plugin copied to clipboard

Allow build numbers to be arbitrary strings or null

Open marcelstoer opened this issue 6 years ago • 12 comments

This required changing the buildNumber data type from long to String. @CDPrete this fixes your issue #75. Hence, your feedback is appreciated.

marcelstoer avatar Sep 19 '19 13:09 marcelstoer

@danielflower I have more new features coming that are based in this branch. Hence, I would appreciated if you could provide guiding feedback soon, thanks.

marcelstoer avatar Sep 24 '19 14:09 marcelstoer

@danielflower Besides this and #94 I have more improvements in mind for this plugin. Yet, I must say not even getting any feedback on PRs is very de-motivating. What are your plans with this project if I may ask?

marcelstoer avatar Oct 08 '19 09:10 marcelstoer

Hi Marcel, sorry I know it's frustrating to not get feedback. I just really don't have much time to go through things very often. Merging a pull request is a commitment on my behalf as I have to understand what is happening on code I have touched for years and then support those changes going forward, so I need to find enough time to go through changes. (The smaller the change, the easier to do.)

As for this change, this feels like quite a bit of a change from the current logic of "the build number gets incremented". I don't really understand what happens when it tries to increment a string.

danielflower avatar Oct 09 '19 14:10 danielflower

I'll be happy to answer any questions - just keep'em coming.

As for this change, this feels like quite a bit of a change from the current logic of "the build number gets incremented".

Most of changes are caused by the long-to-String switch for the build number (many method signatures, lots of tests). And then there are the new tests of course.

I don't really understand what happens when it tries to increment a string.

To me the only really interesting corner case is if you switch from numeric build numbers (past) to alpha-numeric (now and possibly for the future) when the business version remains as-is.

findTagWithHighestBuildNumber is interesting here. See https://github.com/danielflower/multi-module-maven-release-plugin/pull/90/files#diff-98bd0c93f45924a297818a9f54e14d63R200 The "highest build number" is really just that: the largest non-alpha-numeric build number.

Or nextBuildNumber at https://github.com/danielflower/multi-module-maven-release-plugin/pull/90/files#diff-55ca5a18708be7ad8ecb7bceb23cb697R61. Skips over any non-numeric previous build numbers. Keep in mind that this method will only be called if you don't set a build number yourself!

marcelstoer avatar Oct 09 '19 14:10 marcelstoer

All review comments addressed from my side.

marcelstoer avatar Oct 22 '19 08:10 marcelstoer

This required changing the buildNumber data type from long to String. @CDPrete this fixes your issue #75. Hence, your feedback is appreciated.

It looks promising to me.

cdprete avatar Oct 30 '19 21:10 cdprete

Can I do anything to help land this on master?

marcelstoer avatar Jun 10 '20 15:06 marcelstoer

The reason I've not merged this so far is that in my mind this is a fundamental change to the plugin. In my mind, the plugin generates build numbers for you by adding 1 to the previous number. I'm probably missing something though.

Maybe some examples of build numbers in the pom.xml vs build numbers in a release?

e.g. currently if the pom version is 1.0-SNAPSHOT then the first build is 1.0.0 and the second is 1.0.1. This is what I can remember about this plugin. So what scenarios does this pull request cover?

danielflower avatar Sep 06 '20 15:09 danielflower

in my mind this is a fundamental change to the plugin

Don't worry, it's not 😄

The original behavior does not change at all. Unless you (as a user) explicitly want so the plugin behavior remains as-is. There are situations, though, where this strict build number increment is undesirable. You could want your build number to be "A" or "foo" - or nothing at all. The Javadoc for buildNumber() in BaseMojo.java has a long description for the behavior. It goes without say that if you chose "A" in the previous release the plugin won't be able to automatically increment that for the current release.

Hence, to allow such flexibility I had to change the data type from long to String. That's the cause for many of the changes you see in this PR.

marcelstoer avatar Sep 07 '20 06:09 marcelstoer

Hi, I don't know if you are still interested in this, so long afterwards, but if you want to get it merged I think all it's missing is an explanation of when this can be used, why it would be used, and what happens to build numbers after using it (this lack of understanding on my behalf is why I took so long to take the time to understand this, so it's probably confusing to others too).

I think a section on https://danielflower.github.io/multi-module-maven-release-plugin/usage.html would be good.

Something like this?

Specifying custom build numbers

If you want to make a release that has version number with a string in it because ________ To do this, set the parameter like so: example

Perhaps work in the clear explanation that @CDPrete originally wrote on #75

  • if the buildNumber is specified:
    • if the string is not null and not empty, then attach the delimiter
    • try to parse it as Long and, if it works, just use parsed-value + 1
    • if the parsing fails, use the specified string
  • else if no buildNumber is specified:
    • just use the business version alone

(Does the above logic hold for this PR?)

The documentation needs some concrete examples, e.g. if I have 1.0-SNAPSHOT and set the build number to "BETA" ill the new version by 1.0.BETA? And then if I do another release with no build number, is the verison 1.0.0? Is the next release after that 1.0.1 or is that 1.0.BETA version having an influence?

I must say though, as a user of mvn versions:display-dependency-updates I find it really annoying when people don't use numeric version numbers.

danielflower avatar Feb 14 '21 09:02 danielflower

If I got your last comment right then I guess the "lack of understanding" on your behalf is a (maybe) simple misunderstanding. This change here only affects the build number i.e. anything you specify after the - ideally semantic - version. This is limited to the part that the plugin modifies by automatically incrementing. So, for your example it wouldn't be 1.0.BETA but rather 1.0.0-BETA.

marcelstoer avatar Feb 14 '21 14:02 marcelstoer

Yeah, I guess I never understood this PR then.

So if I have 1.0-SNAPSHOT and set build number to BETA it becomes 1.0.0-BETA. The next release (with no custom build number) is 1.0.1 etc?

So this is more of a suffix to the version than a build number? Then why not leave it as a long and have a new property: versionSuffix or some-such?

danielflower avatar Feb 14 '21 16:02 danielflower