aleksandr-m / gitflow-maven-plugin

The Git-Flow Maven Plugin supports various Git workflows, including GitFlow and GitHub Flow. This plugin runs Git and Maven commands from the command line.
https://aleksandr-m.github.io/gitflow-maven-plugin/
Apache License 2.0
494 stars 182 forks source link

Get develop version from branch develop during feature-finish #367

Closed AdrienHorgnies closed 1 year ago

AdrienHorgnies commented 1 year ago

Hi,

Currently, feature-finish deduces the version from the name of the feature branch version. This assumes that the version of develop didn't change. I think we can't make this assumption, and should instead fetch the version from the development branch, which will always be correct.

As taking the version from the branch develop is always correct, I think this PR makes #239 obsolete.

closes #147

aleksandr-m commented 1 year ago

Can you create new tests specially for that issue, instead of modifying existing.

aleksandr-m commented 1 year ago

I don't think that always using development version is correct.

aleksandr-m commented 1 year ago

Maybe some parameter to switch which one to use. Or maybe compare versions and choose which one is higher.

AdrienHorgnies commented 1 year ago

Can you create new tests specially for that issue, instead of modifying existing.

Well, I broke the test because they weren't correctly setup. The develop branch had the same version as the feature branches. Which isn't supposed to happen. Develop branch is supposed to have version 0.0.3-SNAPSHOT, not 0.0.3-test-SNAPSHOT. The changes this PR bring are sensitive to this, so I had to change the tests.

I added a test which represents the version of develop increasing after the feature branch was taken.

AdrienHorgnies commented 1 year ago

I don't think that always using development version is correct. Maybe some parameter to switch which one to use. Or maybe compare versions and choose which one is higher.

That's crucial to know. I believe using development version is correct except with the option incrementVersionAtFinish, and in this case, develop version is not used.

The plugin currently computes develop version using the branch name of the feature version, and that may be incorrect, as develop version could have changed since the branch was taken (see test feature-finish-4-it). So this PR makes the plugin retrieve the version from develop rather than computing it, which can only be "more" correct, if my assumptions are correct.

Maybe some parameter to switch which one to use. Or maybe compare versions and choose which one is higher.

That would be only useful if retrieving the version from the develop branch can be incorrect.

I can see two cases where an issue can arise, and none of them make sense:

aleksandr-m commented 1 year ago

Why existing tests init must be modified?

... with the option incrementVersionAtFinish, and in this case, develop version is not used.

You are using development version here: GitFlowVersionInfo nextVersionInfo = new GitFlowVersionInfo(devVersion, getVersionPolicy());

Why do you think developers shouldn't change feature version manually?

AdrienHorgnies commented 1 year ago

Why existing tests init must be modified?

Because the existing tests init.bsh scripts resulted in the following branches and versions :

I believe this is wrong and that the tests should set up the following branches and versions :

The current tests (before my PR) pass because they ignore the true version of the branch develop (0.0.3-test-SNAPSHOT) and remove the feature name (test) from the version of the branch feature/test instead.

Because I modified the plugin to stop ignoring the true version of the branch develop, I need develop's version to be correct (0.0.3-SNAPSHOT).

You are using development version here:

My bad, my earlier statement was incorrect, I confused different PRs I'm working on. It doesn't change that taking the version from develop makes more sense than deducing it from the feature branch version. Please check the new IT test feature-finish-4-it, it shows a use case that the plugin currently fails (without my PR), that it will pass with my PR.

Why do you think developers shouldn't change feature version manually?

Because they're using a plugin to automate the handling of their versions. If they want to do it manually, they should stop using the plugin. It's either manual or automatic, not both.

AdrienHorgnies commented 1 year ago

@aleksandr-m Is this satisfactory, do you disagree, or do you have more questions ?

AdrienHorgnies commented 1 year ago

@aleksandr-m I think I've addressed all your concerns. Do you have any comments ?

aleksandr-m commented 1 year ago

@AdrienHorgnies If the tests are incorrect then let's start with fixing/improving them. Can you create separate PR with corrected tests?

AdrienHorgnies commented 1 year ago

Sure, I'll do that.

As mentioned in another PR, my client parted way with this tool and I didn't have time to finish this PR.