Pick better merge base #1225

JakeGinnivan · 2017-05-21T09:52:46Z

Builds on #1224, includes the test from #1204 and gets it passing.

This change allows GitVersion to better decide which branch the current branch was likely branched from. Have added some new configuration options and updated the related docs.

Would love any other ideas on how to solve this problem better.

adamralph · 2017-05-21T10:34:00Z

docs/configuration.md

+|/
+*
+*
+* (master)


It's not obvious to me what each branch label refers to in this diagram. I think the branch labels should really be shown next to the head commit of the branch.

Specifically, I'm not sure if you mean this:

* feature/foo * master | release/1.0.0 // head of release branch behind head of feature branch | _________/ |/ | |

or this:

* feature/foo, release/1.0.0 // head of both branches is the same * master | | _________/ |/ | | |

Nice idea, have fixed

It was

* release/1.0.0 * feature/foo | ________________/ |/ * * * (master)

adamralph · 2017-05-27T11:18:28Z

docs/configuration.md

+
+Or put more simply, what branch was created first, `release/1.0.0` or `feature/foo`.
+
+To resolve this issue we give GitVersion a hint to how we normally do our branching, feature branches have a value of:


Perhaps the fault is on me, but this sentence, and the following list, do not make any sense to me. I've honestly no idea what you mean by "feature branches have a value of", and what information the list is designed to convey.

I think what's meant is what the default configuration for feature branches is. So doing GitVersion init should give you a GitVersion.yml containing the list below. Perhaps a better description would be:

To resolve this issue, we give GitVersion a hint to how we normally do our branching. Feature branches are by default configured to have the following source branches:

Rewrote it, hopefully makes it a bit clearer

adamralph · 2017-05-28T18:36:20Z

docs/configuration.md

@@ -243,6 +243,47 @@ values, but here they are if you need to:
 ### regex
 This is the regex which is used to match the current branch to the correct branch configuration.

+### source-branches
+Because git is a directed graph GitVersion sometimes cannot tell which branch the current branch was branched from.


Because git is a directed graph...

I'm guessing that a significant number of people won't know what this means, and it seems redundant. Perhaps remove it?

I think it's valuable to point out that sometimes GitVersion can't figure out the origin of the current branch. Whether that's because it's a DAG or not is perhaps not that important.

adamralph · 2017-05-28T18:37:16Z

docs/configuration.md

+   - Add a commit to both release/1.0.0 and feature/foo
+   - feature/foo is the base for release/1.0.0
+
+Or put more simply, what branch was created first, `release/1.0.0` or `feature/foo`.


Suggest:

Or put more simply, ~~what~~ you cannot tell which branch was created first, release/1.0.0 or feature/foo.

adamralph · 2017-05-28T18:38:55Z

docs/configuration.md

+
+Or put more simply, what branch was created first, `release/1.0.0` or `feature/foo`.
+
+To resolve this issue, we give GitVersion a hint to our branching workflows by telling it what types of branches it can be created from. For example feature branches are by default configured to have the following source branches:


Suggest:

To resolve this issue, we give GitVersion a hint to about our branching workflows by telling it what types of branches it a branch can be created from. For example*,* feature branches are*,* by default*,* configured to have the following source branches:

adamralph · 2017-05-28T18:39:50Z

docs/configuration.md

+
+To resolve this issue, we give GitVersion a hint to our branching workflows by telling it what types of branches it can be created from. For example feature branches are by default configured to have the following source branches:
+
+`sourceBranches: ['master', 'develop', 'feature', 'hotfix', 'support']`


Is this list correct? Under GitFlow I was under the impression that feature branches should only be created from, and merged to, develop. And for GitHubFlow, master.

This configuration param is for all flows, GitVersion actually doesn't know about the different workflows, it just has branch configuration which works for both

adamralph · 2017-05-28T18:42:11Z

docs/configuration.md

+
+`sourceBranches: ['master', 'develop', 'feature', 'hotfix', 'support']`
+
+This means that we will never bother to evaluate pull request branches as merge base options, being explicit like this helps GitVersion be much faster too.


Suggest:

This means that we will never bother to evaluate pull request branches as merge base options~,~ and being explicit ~~like this helps GitVersion be much faster too~~ in this way also improves the performance of GitVersion.

adamralph · 2017-05-28T18:43:35Z

docs/configuration.md

+
+This means that we will never bother to evaluate pull request branches as merge base options, being explicit like this helps GitVersion be much faster too.
+
+### is-source-branch-for


Why is this kebab-cased, whereas sourceBranches is camelCased above (but kebab-cased below)?

Kebab case would be more consistent, so I believe the camel casing is a typo?

adamralph · 2017-05-28T18:45:50Z

docs/configuration.md

+### is-source-branch-for
+The reverse of the above setting. This property was introduced to keep it easy to extend GitVersion's config.
+
+When you add a new branch type this allows you to specify both source-branches for the new branch, and also add the new branch to existing branch configurations. For example if you create a new branch called `unstable` you could set a value of `['release', 'master', 'feature']` etc. 


Again, it might be me, but I don't get this at all.

@adamralph: It's not just you. I don't get how the branch unstable can be the source branch for release, master or feature. 🤔

One more point is to not use position depending things like "above setting". It's always better to refer to the setting by name, so that later description will not be broken due to changed sorting.

zvirja · 2017-06-13T12:10:12Z

@JakeGinnivan Do you currently have any time to follow up? 😳 I have yet another thing that I need improve in this direction, so I would like to base my PR on your one. Given that the documentation stuff is under the discussion only, it looks not too much left 😉

JakeGinnivan · 2017-06-18T02:43:34Z

@zvirja sorry, will get the feedback addressed and just merge

This allows GitVersion to know what branches it should take into account for calculating merge base. This locks it down a little more, but is an easier to explain behavior than commit counting randomly resetting because GitVersion cannot effectively figure out which branch the current branch was branched off.

JakeGinnivan · 2017-06-18T02:59:53Z

Have addressed all the feedback, thanks all. Going to merge once builds are green.

adamralph · 2017-06-21T09:42:21Z

@JakeGinnivan the final revision LGTM.

The only thing I still don't understand is how unstable can ever be the source branch for branches such as master, develop, hotfix, release or support as demonstrated in the is-source-branch-for example. I realise it's only an example, but is it demonstrating something that almost no-one would ever want to do? Or am I misunderstanding something?

This was referenced May 21, 2017

Invalid release version when some branch created from it #1204

Closed

Feature/fix mainline feature branch bump #1224

Merged

JakeGinnivan force-pushed the feature/pick-better-merge-base branch from f7f14e7 to 6f3e788 Compare May 21, 2017 09:59

adamralph reviewed May 21, 2017

View reviewed changes

adamralph reviewed May 27, 2017

View reviewed changes

adamralph reviewed May 28, 2017

View reviewed changes

JakeGinnivan added 8 commits June 18, 2017 10:58

Made sure it's still easy to add new branches to configuration.

28d1fd5

Reverted a logging change

9e53628

Updated docs to make clearer

ca60c5c

Added breaking change note

c4b1d16

Added test for config outdated error

80d6f5b

Trying to make docs a bit clearer

cfc3cae

PR Feedback addressed

de2d3f2

JakeGinnivan force-pushed the feature/pick-better-merge-base branch from 6a075ec to de2d3f2 Compare June 18, 2017 02:59

JakeGinnivan merged commit 00c26d4 into GitTools:master Jun 18, 2017

JakeGinnivan deleted the feature/pick-better-merge-base branch June 18, 2017 05:46


		Or put more simply, what branch was created first, `release/1.0.0` or `feature/foo`.

		To resolve this issue we give GitVersion a hint to how we normally do our branching, feature branches have a value of:


		Or put more simply, what branch was created first, `release/1.0.0` or `feature/foo`.

		To resolve this issue, we give GitVersion a hint to our branching workflows by telling it what types of branches it can be created from. For example feature branches are by default configured to have the following source branches:


		To resolve this issue, we give GitVersion a hint to our branching workflows by telling it what types of branches it can be created from. For example feature branches are by default configured to have the following source branches:

		`sourceBranches: ['master', 'develop', 'feature', 'hotfix', 'support']`


		`sourceBranches: ['master', 'develop', 'feature', 'hotfix', 'support']`

		This means that we will never bother to evaluate pull request branches as merge base options, being explicit like this helps GitVersion be much faster too.


		This means that we will never bother to evaluate pull request branches as merge base options, being explicit like this helps GitVersion be much faster too.

		### is-source-branch-for

Pick better merge base #1225

Pick better merge base #1225

Uh oh!

Conversation

JakeGinnivan commented May 21, 2017

Uh oh!

adamralph May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asbjornu May 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zvirja commented Jun 13, 2017

Uh oh!

JakeGinnivan commented Jun 18, 2017

Uh oh!

JakeGinnivan commented Jun 18, 2017

Uh oh!

adamralph commented Jun 21, 2017

Uh oh!

Uh oh!

adamralph May 21, 2017 •

edited

Loading

asbjornu May 28, 2017 •

edited

Loading