Issue #2148: Limit number of commits traversed #2149
Conversation
Limit the number of commits traversed when trying to find version from tags and merge messages to 2 (each). Solves #2148
|
I wonder if 2 messages is enough for us. Also in the issue is mentioned to have this configurable, I'd prefer if we introduce this limitation also to have that option for configuration. @erikbra would you mind adding that as a configuration? I prefer to have it on the config file, not in the command line |
|
I wouldn't mind adding it as an option, no problem. But, I would like to understand. Could you think of a scenario where you would need more than 2? Is it with more than 2 actively developed branches with different versions? Say, a 1.x branch and a 2.x branch and a 3.x branch? Also, would you like one config setting per strategy? One for MergeMessage and one for Tags? |
|
@asbjornu I think you can tell more on that |
|
As long as the order of the tags and messages is correct, I don't think we need more than 2. It would be interesting to see if this actually breaks anything before we introduce configuration for it. |
Tests that we have a passing so I guess it works |
Are the tests we have complicated enough and with sufficient amount of commits and tags to be sure? |
I think we need to add couple of tests with more than 3 tags/merge messages each |
Agreed. Is that something you please can add to the PR, @erikbra? |
|
I can definitely write some more tests. But can you think of a scenario where you might need more than two? I need to see the scenario to write some tests. Which branching strategy/set of branches would that be? Are we talking about three actively maintainted branches with release tags, and merges into master? |
|
To be honest, I can't think of a sensible branching strategy where that would be the case myself, but after maintaining GitVersion for a few years, there's been so many bizarre branching strategies and flows presented that people want and often manage to bend GitVersion to support that it's very hard to guarantee GitVersion not being used that way today. |
|
Hehe... see your point. But, a test would then be a branch with more than 2 merge messages/tags on the branch, is that it? |
|
Yes, I think so. Perhaps with some strange ordering just for the heck of it? |
|
@erikbra do you think you can add the tests quickly? I plan to release version 5.2.0 and wonder if we postpone the release to include the changes or we include them in the next version? |
|
Yes, I'll try do add them today or tomorrow |
|
@arturcic I added a test, and needed to up the number of commits to 3 to make the test work. I think the example is a bit contrieved, having 3 actively developed release branches. But I upped it to 5, just to be safe. It's still a lot better than traversing all commits. I tried to fint a way to make it a config, but I couldn't figure out the config system. So, my suggestion is to merge this as is now, and, if we get reports of issues, introduce a config variable in a patch release of GitVersion. I think it's a very uncommon scenario, but of course, people have different scenarios. What do you think? |
|
@erikbra ok I think we merge and if any issues with it we'll release a patch |
|
thank you so much for your contributions 👍 @erikbra |
|
Lovely. No worries. Thank you for your work with Gitversion. Use it every day. |
Limit the number of commits traversed when trying to find version from
tags and merge messages to 2 (each).
Closes #2148