Standardize version number generation

[CharliePoole]

The versions now being generated by GitVersion for NUnit.System.Linq follow GitVersion's internal standards, which are not the same as our own.

The latest master build would have been something like 0.6.1-CI-23 by our current standards. However, the GitVersion generated numbering is 0.6.1+23.build.62 and the package published to MyGet is version 0.6.1. I'm assuming the internal AssemblyVersion is also 0.6.1 as we can't actually tell any longer without examining the assembly.

All of this is OK and expected at this point. NUnit.System.Linq was chosen for us to work with GitVersion because it is only used by us. However, we should conclude our experiments quickly and remove any extraneous packages form MyGet.

This issue will be used to decide on our versioning going forward and will also apply to other packages.

@asbjornu I'll get this started by creating a branch and giving you access to it.

[CharliePoole]

@nunit/core-team I'm somewhat blocked by not having a standard naming convention, so I'll lay out a proposal...

Branch	Feed	Default Version	Proposed Version
master	NuGet	3.4.1, 3.5.0-beta1	(same)
develop	MyGet	3.5.0-unstable-nnnn	3.5.0-dev-nnnn
feature/*	AppVeyor	3.5.0-feature/SomeFeature	3.5.0-SomeFeature
bugfix/*	AppVeyor	3.4.3-bugfix/SomeBug	3.4.3-SomeBug
pull/*	None	3.4.1-PullRequest-nnnn	3.5.0-pr-nnnn
release/*	AppVeyor	3.5.0-beta-nnnn	3.5.0-rc-nnnn
hotfix/*	AppVeyor	3.4.1-beta-nnnn	3.4.1-rc-nnnn
support/*	None	NA	NA

Note: Not including PRs in the AppVeyor feed is a proposed change from what we now do.

[CharliePoole]

I should have mentioned one unstated assumption in my last comment... I'm assuming that the existence of a develop branch will lead us to phase out alphas and betas. We may want RCs, which are then released without change if all goes well, but the function of beta releases is pretty much taken over by the existence of the develop branch.

If we do phase out alphas and betas, then we may do it sooner on smaller projects and later on more complex ones like NUnit itself.

[CharliePoole]

I updated the proposed pre-release formats after trying the first set out on GitVersion. The updated values are more in line with what GitVersion expects.

Still waiting for some input on whether it's OK that these are only partially ordered. Specifically, the AppVeyor feed would continue to require selection of the exact build needed since there is no meaningful definition of "latest."

[OsirisTerje]

The version string suffixes should be in alphanumerical sequence, otherwise *-branch will be "Later" than *-beta, I am not sure that is what we want. And -dev will be later than both of these, so perhaps good to name packages created off the dev and feature branches as` -alpha ?

[CharliePoole]

I added the GitVersion default values to the table, in order to clarify things a bit.

I understand the point about ordering pre-releases, but for the moment, I want to take the contrary view, just for the sake of discussion. I'm not totally wedded to this approach, but I'd like to see it explored before we decide. I was following two principles...

1. Ordering need only apply within a feed
2. The Appveyor CI feed doesn't have to be ordered

If we change either of those, then we have to change the suffixes. However, I would argue that a truly meaningful order is not possible across all feeds, so long as we are publishing work in process. I'll take this one feed at a time.

NuGet We only put real public releases out to NuGet, which means and we only create those releases out to the master branch. This feed will always be in order.

MyGet As proposed, we only put actual releases of the develop branch on MyGet. The GitFlow default used unstable as the tag and I used dev. I'd be happy to stick with unstable, which I suspect they chose because it's very high in the alphabet. However, all the releases on this feed would have the same tag, so it doesn't much matter.

If we wanted to call these beta releases, we could do that. But in that case, I would suggest putting them out to nuget as pre-releases, rather than using MyGet. In order to do that, we would have to stop doing the other sort of beta that we now do - i.e. a beta that appears right before the release is ready. IOW, we would be in a continuous beta state for the next release starting right after any master release.

AppVeyor The AppVeyor feed is already unordered, at least in any meaningful way. That is, it's properly ordered because all the packages use the CI prefix and the appveyor build number, but the order has no meaning except that it's chronological. It includes different branches and PRs, built for different purposes. To use it, you have to cherry-pick the branch you want. This is OK since it is mostly used by us and sometimes by users testing out a particular bug. But it really has no useful "latest release" If you were to use it and install latest, you could as easily get a build of some proposed feature from a user as anything really meaningful.

Consequently, I haven't worried about keeping the appveyor build ordered in the proposal.

Combining feeds The only meaningful combination of feeds is between NuGet and MyGet. That's because you may want to get the latest stable release of one component and the latest unstable of another. This will work because at any given time the NuGet feed has the latest build of the current minor version of each component, while the MyGet feed has the next minor version.

---

Anyway, that's the logic that led me to do it as I did. I just want to be sure that logic is understood before we decide to go another way.

[CharliePoole]

Setting aside the labels used for the moment... I ran a series of 12 tests in this repo, creating fake features and hotfixes and merging them to see what happened. The base for all this was master at version 0.7.0. Here are the results, with a few notes attached. You can see the jobs and artifacts on AppVeyor.

1. Created feature/TestFeature1 branch, added a file and pushed it. Appveyor job: 0.7.1-TestFeature1.1+1.build.73 Package version: 0.7.1-TestFeature1-1
2. Created features/TestFeature2 branch, added a file and pushed it. Appveyor job: 0.7.1-TestFeature2.1+1.build.74 Package version: 0.7.1-TestFeature2-1 Note: The 's' on features was an typo, but didn't matter because of the regex used to identify the branch.
3. Modified feature/TestFeature1 branch Appveyor job: 0.7.1-TestFeature1.1+2.build.75 Package version: 0.7.1-TestFeature1-1 NOTE: Expected 0.7.1-TestFeature1-2. Does this need fixing?
4. Merged feature 1 into develop Appveyor job: 0.8.0-dev.5.build.77 Package version: 0.8.0-dev-5 NOTE: It's '5' because I was fooling around in the branch earlier
5. Merged feature 2 into develop Appveyor job: 0.8.0-dev.7.build.78 Package version: 0.8.0-dev-7
6. Created release-0.8.0 branch from develop, added changes.txt and pushed. Appveyor job: 0.8.0-rc.1+1.build.79 Package version: 0.8.0-rc-1
7. Created a pull request for release-0.8.1 branch Appveyor job: 0.8.0-pr.10+9.build.80 Package version: 0.8.0-pr-10
8. Used GitHub online interface to merge the request to master Appveyor job: 0.8.0+0.build.81 Package version: 0.8.0 NOTE: Earlier merges were done locally and then pushed
9. Created branch hotfix/0.8.1 from master, added a file and pushed Appveyor job: 0.8.1-rc.1+1.build.82 Package version: 0.8.1-rc-1
10. Created a PR for the hotfix Appveyor job: 0.8.0-pr.11+2.build.83 Package version: 0.8.0-pr-11 NOTE: I thought this would be 0.8.1, but apparently not. Why?
11. Used GitHub online interface to merge the hotfix to master Appveyor job: 0.8.1+0.build.84 Package version: 0.8.1
12. Manually merged the hotfix to develop Appveyor job: 0.9.0-dev.1.build.85 Package version: 0.9.0-dev-1

---

The sequence needs review to ensure that incrementing is taking place as expected. Some specific issues are noted on the affected item.

The only publishing that is currently being done is for master. I think deployment has to be phased in after we convert to using GitVersion. The 0.8.0 and 0.8.1 master builds were deployed on MyGet.

[ChrisMaddock]

Apologies for referring to something a few days old...

Note: Not including PRs in the AppVeyor feed is a proposed change from what we now do.

Does this potentially limit how we can work on the core project, or is there a workaround I've missed?

My situation: I'm currently working on a framework change which will require a couple of new LINQ additions. To do that, I want to add them to this assembly, and get a 'temporary' package that I can base my framework work off. (From a nuget feed rather than a local package, so I can submit a working [WIP] PR to the framework.)

My only way to currently do that seems to be to submit a [DoNotMerge] PR to this repo, and then use the package from the appveyor feed. If we don't have PR packages, would there be another way around this problem?

This specific case could potentially be solved by teams having branch access to all dependency repo's, but would that then equally limit a non-team members ability to contribute?

[rprouse]

@ChrisMaddock that is a good catch on your part, I missed it. I am using that ability constantly working on the .NET Core projects at the moment. I make a fix in one repo, as you said, create a PR, then pull in the NuGet package in the second repo and test or use the changes.

We are adding a single NUnit wide AppVeyor NuGet feed. I think it is fine that PR builds are not pushed to it, but we could work around the problem by continuing to maintain project specific feeds that are only used for development. @CharliePoole was this the intention?

[CharliePoole]

@rprouse @ChrisMaddock

This question is quite a can of worms. :-)

@ChrisMaddock From your post I take it that you are OK doing local testing but you want to be able to submit a PR that will build on AppVeyor. So, for example, you may be working on a change in the framework that depends on some new class being in NUnit.System.Linq. Have I got that right?

Further, I take it that you are already set up with a local feed where you can test things before submitting.

So..., this introduces quite a few issues... :-)

1. The current AppVeyor feed cannot be used that way unless you were to submit the PR for the framework project specifying a specific build in packages.config, e.g.: version=[0.9.0-CI-xxx-PR-yyy]. That's because the feed, while ordered, is not ordered in any meaninful way. If you just let it get the "latest" then it could be superseded by somebody else's branch or PR build that didn't include your change. If you did use the exact version in the PR, then you would have to remember to change it back to a correct version before merging, which seems risky.
2. The way we envisioned making changes across different repositories - and the way I have been doing it for the NUnit 3 Test Adapter - is that the underlying change has to be made first, merged and published to either MyGet or Nuget. It can be a pre-release but it has to be in ordered sequence on the same branch - master right now, develop when we implement it. This is harder than having everything in one project and we knew that when we decided to split things up. But we decided to accept that obstacle with the idea that only a minority of changes would involve changing some other package before it could be made. That seems to be true once a package is developed. It's less true for completely new things like the .NET Core runner or (in the recent past) the NUnit 3 Test Adapter. We hope that the benefit we get from separation will outweigh the difficulties.
3. For the specific change you want to make, you should submit an issue on this project. Explain what you want to do and why. We will review that and tell you what we think. Assuming the change is accepted as an issue, then do a PR and get it published. Then go on with the change you want to make. The alternatives we will consider to changing NUnit.System.Linq are (1) excluding the new feature (I assume it's a framework feature) from the 2.0 build or (2) asking you to implement it without use of System.Linq.
4. I can't be sure what you are working on, but _if _ it's the issue about expressing properties as lambdas, my inclination would be to exclude it from the 2.0 build. If a user has decided to write tests using .NET 2.0 rather than 3.5, then they probably aren't expecting to use lambdas in the interface with NUnit. They always have the option to target 3.5 in the tests, even if the SUT targets 2.0. In general, I think we can use (and do) System.Linq features in our implementation of the 2.0 build but should keep them out of the user interfaces.
5. Back to the more general question that underlies this IMO: "What is each feed for?" Here's my take on it...
   • NuGet is for real production releases to users, including any alphas, betas or rcs that we actually intend for users to download and try out.
   • MyGet is for our own internal use where one of our packages depends on another and for users who want to get a continuous development feed. It actually supports multiple feeds if we find the need to have them and we could set one up that way as Rob suggests. So far, we just have one feed.
   • AppVeyor is for testing, generally by us but sometimes by users who select a very specific package whose name we give them. Unless we were to restrict it to master builds only, its contents are mixed and "latest" really has no meaning.

This is pretty important stuff with a lot depending on it, so please argue with me if what I have written doesn't seem right. We want to get this right!

[CharliePoole]

I'll add one more thing and then wait for comments. :-)

If somebody is working on a feature that requires constant changes to two independent packages, going back and forth between them. The approach of completing the lower level package and publishing it first doesn't work. This has been the case with the gui, for example, where I constantly discovered new things I wanted out of the engine.

The way I dealt with that was to pull the engine project into my solution locally and work on both. But before I could publish anything for the gui, I had to first make the underlying changes to the engine and get them merged. It's not convenient, as already stated.

If we were happy to continue all our development with a small number of devs, as now, then we could have kept everything together in one big project. We are taking a chance that separate, smaller projects will help us pull in more devs.

[rprouse]

@CharliePoole I think this is a case of constant changes in two independent packages. Right now, that is happening between nunit.portable.driver and dotnet-test-nunit. I have currently solved the problem by adding the AppVeyor nuget feed for the portable driver to the dotnet test project and consuming specific builds of PRs. It means a slight delay waiting for the AppVeyor build, but it is manageable.

Another workaround is to package on the developer's local machine and add the packages to a local NuGet feed directory. I do that for faster turnarounds. We could work that way, but we should document it.

[ChrisMaddock]

@CharliePoole: Can of worms indeed! Your assumptions about what I'm doing are all correct, for context, the issue is nunit/nunit#26. I'll comment point by point below!

The current AppVeyor feed cannot be used that way unless you were to submit the PR for the framework project specifying a specific build in packages.config, e.g.: version=[0.9.0-CI-xxx-PR-yyy]. [...] If you did use the exact version in the PR, then you would have to remember to change it back to a correct version before merging, which seems risky.

Yes - far from ideal! Risky, and requires enforced additional to-ing and fro-ing - if the person who reviews is happy with everything as it is, they can't just merge as further changes will still be required.

The way we envisioned making changes across different repositories [...] is that the underlying change has to be made first, merged and published to either MyGet or Nuget. [...]

For the specific change you want to make, you should submit an issue on this project [...]

I can't be sure what you are working on, but if \ it's the issue about expressing properties as lambdas

I think these are all valid points for the issue at hand, but perhaps we should be looking at this more with multiple dependencies more generally. NUnit.Linq is a relatively simple example, in that we have a relatively clear idea of the specific missing namespaces required for the implementation.

From the point of view of how I personally would work, it feels like it would slow changes down significantly if I had to wait for a review/merge mid-way through a substantial change. Of course, I could do this all locally, but it feels messy in terms of replacing a NuGet reference with a local one.

The 'least-worst' solution to me, feels like being able to just modify the packages.config to an appveyor-esque version number. The change should be relatively clear to catch in review. Could we potentially also set up a CI task which only uses MyGet/NuGet package restore sources, so a PR couldn't be merged with Appveyor packages in the config?

[CharliePoole]

@rprouse Yes, I use a local repo myself now when working on the test adapter. But that's basically past the "back and forth" stage now. Since all the projects are on my machine and adding projects to a solution doesn't copy them, I find a master solution to be fastest in that situation. And it avoids my creating packages for changes that are more of a "what if" nature and may be abandoned later.

I think the way you are working now - using the AppVeyor feed - works well because you are the only one committing to those packages. It might be a problem if you were depending on the engine, for example.

[CharliePoole]

@ChrisMaddock Yes, NUnit.System.Linq is both a simple example and is also completely different from all other packages in that it is only published for our own use. Additionally, it's not our code so we didn't expect to change it once created. We made it a package rather than keeping it in the project because we expect to use it in our engine build, which is about to be separated.

I think we have two entirely separate use cases to deal with:

1. New development of a kind that implies entirely new features in the referenced package - the "back and forth" development that Rob and I are talking about. In such cases, you may not even know in advance exactly what features you want in the lower-level package.
2. More "normal" development where you need an extra feature in a well-established and stable package. Usually, you can specify in advance what that feature is and if not you can experiment locally until you are ready to specify it. I think this is probably the case you are dealing with.

I think the elephant in the room here is that we actually want to slow down some kinds of changes. If a package is being depended on by several different using packages, then we need to be sure a change for the benefit of one of them won't mess up the others. That's what the cycle of issue-discuss-develop-merge is all about. When needful, we can do it fairly quickly, but of course, it's usually a matter of days, not hours, unless you and the reviewer are in just the right time zone relation to one another.

To put it in perspective, here are some changes to the test adapter that necessitated (and waited for) engine or framework changes:

• Getting accurate file and line number locations in the adapter required adding fields to the representation of a test in the framework.
• Displaying immediate output in the Output window required making the framework actually produce such output in the first place, rather than bundling it with the Test Result, and changing the engine to allow passing the output on.
• Recognizing special error conditions in the adapter required adding new custom exceptions to the engine.

Each of these items resulted in two separate issues, separate coding and tests and separate merging. It definitely would be faster if we didn't have to do all this, but with an application in use by hundreds of thousands of developers, I've always felt that a level of care required.

In particular, the creation of an issue for each item is aimed at actually saving you time. If it turns out that what you want to do isn't supported by everyone else, it feels much more efficient to have you type in a paragraph's worth of info as compared to writing the code first. I'll grant you that this doesn't always feel more efficient to a developer who would rather be coding than explaining. :-)

Others may have different views. Here's mine...

1. We want to retain a level of control and care over the packages we depend on that is even higher than what we demand of ourselves for the top-level packages, because they can affect more things.
2. So long as we can meet the standards of item 1, then we want to make the process as efficient as possible. I'd be in favor of creating some feed to which you could commit your changes and then use them if we could figure out a way to trigger use of that feed in our CI build based on some info we pass to it. That implies something that comes from GitHub, like the type of build or the name of the branch or a tag that you add... We don't want to change any files if we can avoid it because we would have to change them back.

[CharliePoole]

Back to the original question, after a wide digression. @ChrisMaddock Why do you prefer to use the PR build over the branch build? The reason that I suggested dropping PR builds is that it didn't seem to have any particular use for testing purposes. Maybe I missed something?

[ChrisMaddock]

@ChrisMaddock Why do you prefer to use the PR build over the branch build?

The core team doesn't (currently) have branch access to this repository. That was what I meant in my original post by a team having branch access to all dependency repos - although that still wouldn't solve the problem for non-team contributors, which it feels like we should also be able to support.

I agree there's value in slowing down large changes. However, it feels like the additions I would want to make in NUnit.Linq would be best reviewed in context of the 'actual' changes I wanted to make in the Framework. It may be that with review, we decide the change the framework implementation, and no longer need the NUnit.Linq changes, or alternatively need further additions.

(Mostly unrelated, I think you make a good point that this particular feature is best just excluded from .NET 2.0 - but I'll move that discussion over to the original issue!)

[CharliePoole]

@ChrisMaddock AH! Well that's just an artifact of the current state of flux of our team setup, particularly as regards to what NUnit.Core means now and will eventually mean. It's a matter of about 60 seconds for me to give you write access to NUnit.System.Linq, completely apart from the team structure.

Contributors are a different matter, but it's hard to imagine a contributor starting off with something so complex that they needed to do multiple package updates. If it happened and they seemed credible candidates, we would probably make them team members of some kind.

NUnit.System.Linq is an exception in all this. It's not assigned to a team because we don't want people to start writing code for it. Rather, we would import additional types if needed.

I'll join you on #26 for more discussion. :-)

[ChrisMaddock]

Ok, sure.

So, to play devil's advocate, if using branch packages IS the right way to approach this problem, then would there be any issue also pushing PR packages to the same feed?

It would be rare for someone to need one, but if it's just a matter of which CI webhooks are enabled, seems worthwhile for the odd occasion they're useful?

[CharliePoole]

Well, it would depend on the purpose. My idea of dropping PR builds from the AppVeyor feed was (1) to simplify it, since it's one of three different kinds of things mixed in the same feed, and (2) because I can't figure out what a PR is particularly good for.

When you push a change to a pull request, you get two builds published, one for the branch and one for the PR. If it's approved, you then get a third build, this time of master. If you do it fast enough, it's probably identical to the PR build.

I'm thinking that we probably don't want it published anywhere if it's not approved. And if it's approved you can get that master build, which appears on both AppVeyor and MyGet. Is there a use case where you want to get the PR build rather than the branch before we have decided whether to merge it?

I'm not opposed to it but I am against doing it just in case we might need it. I'd like to have some idea of what we may need it for.

[ChrisMaddock]

The only use case I was thinking of is when a user doesn't have branch access. In that case, waiting for review/merge before being able to move on isn't ideal.

I agree it sounds like it would be a rare situation however - it's just a question whether it's worth preventing that, just to simplify the Appveyor feed.

[asbjornu]

Sorry for being away for so long. I'm on vacation for 4 weeks now, but are a bit online now and then. Is there anything you need from me in this thread or are we just down to the discussion of which package feeds we should have, now? Please let me know if there's anything I need to do as I don't have time to read the entire discussion until august. 😄

[CharliePoole]

@asbjornu Thanks for letting us know. You're right, I think it's now a matter of our deciding what we want rather than how to do it.