Allow build numbers to be arbitrary strings or null
This required changing the buildNumber data type from long to String. @CDPrete this fixes your issue #75. Hence, your feedback is appreciated.
@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.
@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?
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.
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!
All review comments addressed from my side.
This required changing the
buildNumberdata type fromlongtoString. @CDPrete this fixes your issue #75. Hence, your feedback is appreciated.
It looks promising to me.
Can I do anything to help land this on master?
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?
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.
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.
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.
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?