Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to configure GitVersion to fit our needs #988

Closed
maxraab opened this issue Aug 8, 2016 · 23 comments
Closed

How to configure GitVersion to fit our needs #988

maxraab opened this issue Aug 8, 2016 · 23 comments

Comments

@maxraab
Copy link
Contributor

maxraab commented Aug 8, 2016

Suppose you have a big community and many contributions. Every contribution starts with a feature branch from master and after it's finished gets merged back to master. (Normal GitHub Flow). The main difference is the release stragegy:

If a release is sheduled - a new release branch (e.g. 8-1-stable, 8-2-stable, ...) is created from current master. From this time only some contributions gets cherry picked from master into the stable branch. All release candidates (e.g. 8.1-rc1, 8.1-rc2, ...) where tagged on this stable branch.

If the Version is ready to ship, the Version gets tagged but the release branch will not merged back into master. (To support up to 10 versions at the same time).
From this time, the commits on master should have a version similar to: "next-version"-pre.

@asbjornu
Copy link
Member

asbjornu commented Aug 8, 2016

@maxraab Could you please provide a more visual example of how your Git graph looks like? A simple Web Sequence Diagram would do.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 8, 2016

untitled

As you can see, we have no tags directly in master.

/cc @asbjornu

@DanielRose
Copy link
Contributor

You will need to use the feature #832 for this to work (currently only in the preview releases).

For configuration (see http://gitversion.readthedocs.io/en/latest/configuration/): Mark the master branch as is-develop. You will also need a custom branch name and tag-number-pattern, or change the name of your stable branches (i.e. release branches) to a standard release-1.1 or release/1.1

@maxraab
Copy link
Contributor Author

maxraab commented Aug 9, 2016

Thank you for your response.

I don't get it.

Is it possible to get one of these szenarios working?
I've added two files (szenario1.cs, szenario2.cs) with our testcases.

@DanielRose
Copy link
Contributor

I couldn't test your scenario, but try a GitVersion.yml with the following contents:

branches:
  master:
    tag: pre
    is-develop: true
  releases?[/-]:
    tag: rc

@gep13
Copy link
Member

gep13 commented Aug 10, 2016

@maxraab @DanielRose NOTE: In order to verify any changes that you make in the configuration file, you will need to clear the GitVersion cache, which you can find here:

image

i.e. within the .git folder of your repository. If you don't clear this out, you won't get any different results.

NOTE: This has been corrected in a newer version of GitVersion, but just wanted to point it out in case you don't see any changes in the asserted version number.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 10, 2016

This is exactly the configuration i've tested in Szenario2.css`: See here

2 / 5 tests will fail because:

Testname: CommitsInFeatureBranchWithSingleFeatureBranch should be "0.10.1-BranchD.1+23" but was "0.1.0-BranchD.1+23"

and

Testname: CommitsInFeatureBranchWithMultipleFeatureBranches should be "0.10.1-BranchE.1+23" but was "0.1.0-BranchE.1+23"

It would be nice to have the branchname in the version string.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 10, 2016

@gep13 Thank you.

I assume that every testcase will get a fresh git repository.

@DanielRose
Copy link
Contributor

Okay, if I understand those tests correctly (they are difficult to read because of all the helper methods which create and/or checkout branches, make commits, etc.):

You created a branch release/0.10 with some commits. Thus, the master branch (marked with is-develop) should pick up the version number 0.10. Then you are in a feature branch BranchD. In that branch, it does not pick up the version number 0.10.

That looks like a bug to me.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 10, 2016

Yep, that's the point.
I thought it would be better to encapsulate some logic to methods to not mess up the test.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 10, 2016

@DanielRose The problem I discovered is, that our feature branches has no branch-configuration. (It is not possible to force this in a big community).

Then there is no branch-configuration found in BranchConfigurationCalculator.GetBranchConfiguration() and a default configuration will be created. After ConfigurationProvider.ApplyBranchDefaults() applies a default config, isDevelop is set to false.

If this default configuration would have isDevelop = true, all tests (including my demo-tests) will pass.

@DanielRose
Copy link
Contributor

There are a bunch of default configurations for various branches. You can see the current config (including all defaults) if you run gitversion /showconfig. Feature branches have a default name of features?[/-]. So if you use such a naming convention for your feature branches (ex. feature/A, feature/B) that could work.

@JakeGinnivan At least as far as I understand it, the feature-branches should not need is-develop for this to work. Or did I misunderstand that?

@maxraab
Copy link
Contributor Author

maxraab commented Aug 10, 2016

@DanielRose As mentioned before, we are a big community with a lot of contributors and we cannot force them all to use such naming conventions like feature/A. This is only possible for release-Branches because all of them are maintained by the core team.

@gep13
Copy link
Member

gep13 commented Aug 11, 2016

@maxraab said...
@DanielRose As mentioned before, we are a big community with a lot of contributors and we cannot force them all to use such naming conventions like feature/A. This is only possible for release-Branches because all of them are maintained by the core team.

There is only so much that GitVersion can do in terms of recognising branches, and applying the necessary configuration to the branches. If you are not in a position to control the naming conventions that are used, then perhaps GitVersion isn't the right tool for your needs.

@asbjornu
Copy link
Member

@maxraab But is the entirety of this community allowed to create new branches in the main repository? Don't each individual developer have his own fork? With "everyone" allowed to do "everything" in a singular repository, I understand why it's hard to enforce law and order, but if that's the case, I'd say focus on getting everyone their own fork and do PRs from them. Then the PRs can be merged into branches following a naming convention in the main repository.

Or am I misunderstanding things completely?

@maxraab
Copy link
Contributor Author

maxraab commented Aug 11, 2016

Users can create branches in the main repository and push their code in. Then they open a pull request and only a core-team can review and merge them back to master.

Same thing with the release branches: naming conventions can be forced and only (~30 40 members) are allowed to cherry-pick and/or merge to stable branches.

You also can create a fork of every single repo and do what you want until you open a pull request.

Our problem: the developers have to combine a lot of dll's from different repositories to one project/application. Sometimes dll's where picked from feature-branches for testing purposes. To check the exact versions it would be nice to have a exact assembly-version.

@asbjornu
Copy link
Member

@maxraab Ok, I'm going to go a bit off track here and point out two things:

  1. It sounds like you should stop allowing everyone to create branches in the main repository. The overhead of everyone having their own fork isn't that high. Most open source projects use this model and it is obviously working great given GitHub's success.
  2. It sounds like you should use continuous integration and a private NuGet server (I can warmly recommend MyGet) to have all branches built and published as easily consumable NuGet packages. This way, people don't need to build anything themselves.

I know these don't solve your immediate problem, but if my assumptions are correct, I strongly believe this is something you should take a hard look at and try to fix. It will make everyone's lives so much easier.

To explain why I point this out, it's because GitVersion requires the Git repository to follow a flow to work. These flows are what I'd consider "best practice" for working with large projects on Git. They do carry with them a lot of baggage, though, like the (highly recommended) use of continuous integration, continuous deploy (or delivery), easily consumable artifacts (NuGet server), often a release manager (such as Octopus Deploy), etc.

Without adopting all of the flow's inherent baggage, you're going to find yourself in a pain spot, which I believe is where you're at right now. We were there a few years ago too and it felt hopeless, but incrementally improving the process and adjusting as we saw issues and learned from others, we're finally somewhere quite comfortable.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 11, 2016

@asbjornu Thank you for this long explanation.

We adopted some guidelines and workflows from GitLab and fit them to our needs. We have a private NuGet Server but no automatic package building.
If one of the core team member reviews a pull request, they review the code and download the artifacts from the ci build to test them into different test-environments. Even when I try to define such naming conventions, there will be branchs that don't fit - and these branches did not get a equal version generated as if the branch would named correctly.

I will think about it,

What's your opinion about the potential bug @DanielRose mentioned here?

@JakeGinnivan
Copy link
Contributor

JakeGinnivan commented Aug 11, 2016

Yep, mark master as is-develop: true using config, use v4-beta, and start naming your branches release/1.0 and release/1.1 instead of 1.0-stable and 1.1-stable then it should work for you. Any branch which is not matched in config is considered a feature branch, so you don't need anyone to name anything special, we just truncate feature/ off the front of the branch when using it for tags.

Also, rather than cherry pick, I suggest you get people to target their feature branches towards a release branch rather than cherry picking. Then you can just forward merge the release branch into master rather than cherry picking the other way around? But if the flow you have works, great.

@maxraab
Copy link
Contributor Author

maxraab commented Aug 11, 2016

@JakeGinnivan Ok, thank you. Is it inteded?

You created a branch release/0.10 with some commits. Thus, the master branch (marked with is-develop) should pick up the version number 0.10. Then you are in a feature branch BranchD. In that branch, it does not pick up the version number 0.10.

Testcases can be found here: Test Case

@JakeGinnivan
Copy link
Contributor

So master is building as 0.10.1, then you branch and the branch it is not 0.10.1?

If that is the case, that's a bug. Should be able to verify with a simple test like this:

fixture.BranchTo("release/0.10.0);
fixture.MakeACommit();
fixture.MakeACommit();
fixture.AssertSemVer("0.10.0-rc.1");

fixture.Checkout("master");
fixture.MakeACommit();
fixture.AssertSemVer("0.10.1");
fixture.BranchTo("MyFeatureD");
fixture.AssertSemVer("0.10.1-MyFeatureD.1");

What is the last assert actually in this case? (I have to head off for a bit now so can't actually write this test for real)

@maxraab
Copy link
Contributor Author

maxraab commented Aug 11, 2016

I put your code into a test and fixed the errors:

[Test]
public void TestJakeGinnivan()
{
    using (var fixture = new EmptyRepositoryFixture())
    {
        fixture.MakeACommit();
        fixture.BranchTo("release/0.10.0");
        fixture.MakeACommit();
        fixture.MakeACommit();
        fixture.AssertFullSemver(_config, "0.10.0-rc.1+2");

        fixture.Checkout("master");
        fixture.MakeACommit();
        fixture.AssertFullSemver(_config, "0.10.1-pre.1+1");
        fixture.BranchTo("MyFeatureD");
        fixture.AssertFullSemver(_config, "0.10.1-MyFeatureD.1+1");
    }
}

The test fails:

Shouldly.ShouldAssertException : variables.FullSemVer
    should be
"0.10.1-MyFeatureD.1+1"
    but was
"0.1.0-MyFeatureD.1+1"

It seems to be a bug.

@asbjornu
Copy link
Member

Is this test still failing, @maxraab? Please test with the latest build and reopen this issue if the problem persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants