Using ESS_GUI as the main branch

mdoucet commented 4 years ago

Background The ESS_GUI branch was originally created to start development on a 5.x version. The master branch continues to be the main branch from which we create 4.x releases. Version 5 is now a released product and Version 4 is meant to be phased out. Production code moving forward is based on the ESS_GUI branch. This creates confusion for people wanting the latest version of the code.

Proposed Plan We need to develop a transition plan to ensure that the 5.x code is in the default branch (the branch people will see when going to github, and the one they will get when cloning the repo without specifying a branch).

Here are some thoughts, please comment:

[x] Document the situation: There are a lot of links on sasview.org for developers, but the 4.x / 5.x situation is not well described. We should clearly spell out the situation.
[x] Links to build servers: Both on github and sasview.org, we can only find the links to the 4.x builds. We should swap over to the 5.x servers, and document the 4.x ones elsewhere if we keep them.
[x] Transition the 4.x build servers: Simply changing the names of the branches will just break the builds. We should create a 4.x branch from the current master and reconfigure the builds to use it.
[x] Transition the 5.x build servers: Once the 4.x builds are out of the way, merge ESS_GUI into master (initially without deleting the original ESS_GUI branch), and create new the 5.x builds to work with master. Once everything is happy delete the old builds and the ESS_GUI branch.

wpotrzebowski commented 4 years ago

On aspect to think of is a release process. The current procedure involves one or more release candidates and we are putting other pull requests (for next release) on hold during this process. If it takes a while it becomes problematic. One potential solution (not sure if best though) is to run develop and master branches with master serving as a release workspace. develop branch can be merged with master once release process is completed.

wpotrzebowski commented 4 years ago

Another possibility is to use master and stable: "Consider origin/stable to always represent the latest code deployed to production. During day to day development, the stable branch will not be interacted with."

rozyczko commented 4 years ago

I think we might want to have all three, actually. master/develop split is preferred, but we need to make sure master contains code which is actually the release. Not RC but the final release. One way to do this is to create additional release or RC or stable branch off develop and fix release issues in that branch. Once it's all ready and tested, release is built off of the current RC branch and it is merged to master. All RC fixes are then of course propagated to develop

mdoucet commented 4 years ago

I like that. Maybe this is an opportunity to add a little process...

rozyczko commented 4 years ago

This might make it a bit more transparent

rozyczko commented 4 years ago

We would decide at what functionality to freeze the given release

branch off Develop to a Release branch
test/fix
merge with master
release tagging master at this point

pkienzle commented 4 years ago

As a visitor to a new project I look at the the commits to the default branch as an indication of project activity, and to check the current state of the "bleeding edge" code. If the default branch is not the development branch then I need to read through the entire branch list (including all the feature branches) and guess which one is the "real" trunk.

Therefore I suggest leaving development on master, and putting code freezes on a separate release branch.

Update the release branch as necessary to fix release blockers (i.e., issues which crash the program or produce incorrect results) and nothing else: no more feature creep causing release delays. Merge the changes back into master when the (short) release process is complete.

BTW, in the diagram above the Develop branch has direct commits without merging from a feature branch. Does this mean it isn't locked?

rozyczko commented 4 years ago

GitHub visitors are usually aware of branches and having separate development branch is very common. Normally master is considered the stable branch, unless you have very small repos with few people contributing.

Creating a more complex branching structure is likely beneficial to the team and will help managing the growing project.

We could always change the activity graph to correspond to develop so the project looks much more alive.

rozyczko commented 4 years ago

BTW, in the diagram above the Develop branch has direct commits without merging from a feature branch. Does this mean it isn't locked?

This is to be decided. develop could stay unlocked but we might have a (written?) rule about not pushing to it until code is reviewed. Or lock it.

pkienzle commented 4 years ago

I checked a bunch of repos looking for the workflow you are proposing and didn't find any.

The following packages use master + a branch for each release, and forks for PRs.

python/cpython
numpy/numpy
scipy/scipy
matplotlib/matplotlib
h5py/h5py
docker/docker-ce
pallet/flask
django/django

cpython/mypy does the same, but uses feature branches as well.

HDFGroup/hdf5 calls the default branch "develop", but is otherwise similar. There are two styles of names that look like release branches (for the same releases!), and there are a bunch of feature branches. It looks like the main development is happening on an upstream "develop" branch. There is a "master" branch but it is four years out of date.

In the above cases, I expect that the master branch to be stable since tests should be done on the fork or feature branch before merging (as recommended by github flow).

The changes I propose are:

branch master into a release candidate, leaving master free to merge new features
only fix severe bugs in the release candidate

Setting up a regular release schedule would also be a good idea, say every six months or so.

butlerpd commented 4 years ago

I think I understand @rozyczko graphic which looks very nice. I'm not sure I fully understand @pkienzle suggestions but given our history the idea of two permanent and parallel developer and "stable release" branches with a release candidate branch being made before each release so that PR don't get backlogged for that reason alone, seems sensible to me? I kinda think I understand @pkienzle original comment, but that is just a question of which branch is default? I would agree that for example when making a PR the default branch it should go against is the development branch (which should then be treated the way we currently treat master?) Also a developer probably wants to pull the code from the actively developed branch by default. If you wanted the release version code wouldn't you get it from the release page download?

So would keeping the nomenclature and workflow suggested by @rozyczko but make the development branch the default (base in github parlance) branch deal with @pkienzle main concern?

Either way I think I understand there will be build server issues so the question would be how is that impacted? it seems however that would be linked to issue #1644 ?

pkienzle commented 4 years ago

Continuous integration workflow with master kept in running state and all work on feature branches. Each release lives in its own branch, so no need for a separate development branch. Keep the release cycle short, with only critical releases. By releasing often (e.g., 6 months with whatever is in master), this will reduce the temptation to stick in one more fix before the release. Users who are need fixes between releases can work with the nightly build of master, which should be working if things are tested before merging.

We may want a short feature freeze before each release (1 month?) where we promise to run integration tests, and only merge critical fixes (i.e., things that produce wrong results or cause program crashes). This isn't required since PRs can be posted against the release branch, but that makes the picture look a little less pretty.

butlerpd commented 4 years ago

Thanks @pkienzle now I see 😃 but this looks just like our current structure (how well we abide by the "soft rules" is another story:-)

One concern though is that it seems that you are suggesting commits directly into the release branch (between release candidate and final release). If ever code should be reviewed before merging I would think it would be on a release branch just prior to a "final release"

Of course now that I look at @rozyczko design I am confused by what looks like a dead end "release branch" while master is marked as where all the releases are?

I would have to think a bit about which one of these schemes seems most likely to me to work best in the reality of our group. And not sure I am the person with the most insight into that either. At least we can now all comment on the proposals? and hopefully come to a decision soon?

pkienzle commented 4 years ago

You can do PRs against the release branch. I updated the diagram.

The main difference from the current workflow is that we will not have PRs stuck in limbo for months while waiting for the next release, even if we continue with our current release culture.

I'm hoping that we can change our culture so that we release every 6 mo with whatever features are on master, only fixing crashes and incorrect results in the release candidates. No GUI tweaks, small enhancements or doc fixes. Anything discovered during testing that is not release critical can go immediately into a feature branch for review and be merged into master so that it will be available for the next release even before the current release is complete.

smk78 commented 4 years ago

Time I chipped in I guess...

What I'm reading above does sound much more like the Mantid model as far as I am aware of it, which releases every 4 months. If I had to test SasView that often as well as Mantid I might go insane (or retire) so 6 monthly releases is maybe ok. But the crucial point is that if you know when the next release will be there is less temptation to cram every last cookie in the jar each time. It's also easier (on paper at least) to plan features to releases.

Worryingly, I also think I understand @pkienzle 's diagram!

butlerpd commented 4 years ago

So studying this a bit more it seems to me the two options are functionally equivalent with the only real difference being that Piotr uses two long running branches (master=all releases and develop = current master) while Paul uses 1 "develop branch" called master and an unlimited number of "release branches?" Conceptually hotfixes (which we've never really done as such I guess but that is probably semantics :-) seem cleaner to me in the gitflows approach but functionally identical to what would be done in Paul's approach? Both provide essentially the same release branching that allows development to continue while fixing bugs in the release branch?

If so, as I see it then: The advantage of Paul's version is it doesn't change the way we currently operate (other than enforcing a more strict release cycle) and may be less confusing for outsiders unfamiliar with gitflows develop branch concept while Piotr's approach is much cleaner in terms of branches and might make it easier to enforce the more strict release cycle in our ragtag group :-)

But I'm not a git ninja -- so I may be missing something

wpotrzebowski commented 4 years ago

Both options work for me but I would agree that @pkienzle suggestion is more similar to how we currently operate. Either way will push the limits of current testing/building infrastructure but I think we can handle it (especially if we turn to GH actions).

wpotrzebowski commented 4 years ago

I've made a test on the dummy repository and the following steps worked

git branch -m master master4.x
git push origin master4.x
git branch -m ESS_GUI master
git push origin master

Should I give it a go? I've been also considering full repository back up with issues, milestones, etc (we all have local copies of code). There is BackHub app https://github.com/marketplace/backhub and they have free 14days trial. It may be worth doing it.

wpotrzebowski commented 4 years ago

Then one will need to make sure that master is the default branch and that all PR will be merging against correct branch.

rozyczko commented 4 years ago

This looks reasonably safe. I agree with the sequence of steps. Maybe we could copy ESS_GUI to another branch just to have the full state of the repo before the swap? Seems easier than external backups.

pkienzle commented 4 years ago

Is this equivalent to a force push? What happens on somebody else's machine, where the hash associated with master is for the wx_gui branch [I don't seem to like the name master4.x], rather than the new master based on qt gui?

I'm guessing when you try to pull from a directory where master is pointing to the wx gui, it will go back to the last common commit and attempt a merge. Off hand I don't know the correct way to resolve the conflict. Is there an equivalent to a "force pull"?

Please make sure we have instructions for pulling the new master before updating the sasview repo.

butlerpd commented 4 years ago

I was worried you were losing all the commit history here which would make life difficult if one wanted to be sure one captured everything committed since last release for example (perhaps to verify the release notes are correct/complete). However from reading the documentation at https://git-scm.com/book/en/v2/Git-Branching-Branch-Management that does not seem to be a problem but the dire warnings about renaming master (as opposed to other things)in the last section is a bit .... scary?

wpotrzebowski commented 4 years ago

After some additional testing I can see that point made by @pkienzle maybe an issue with local merges. I guess the only thing one can do is git pull origin wx_gui instead of git pull? After reading the documentation @butlerpd refereed to I suggest more conservative transition approach:

Master branch freeze
Local master to wx_gui transition: git branch -m master wx_gui
Pushing wx_gui to github repo: git push origin wx_gui (at this stage both master and wx_gui will be available at github)
Fix Jenkins scripts and let people to pull wx_gui branch.
Do ESS_GUI to master transition once we are happy with step 4.

pkienzle commented 4 years ago

You will need to rebase the pending PRs against master after moving (or just merge them before branching 8-)

On reflection, since master is locked, (most) developers will only mess up their local machine so much less concerning.

Branching ESS_GUI to "main" (the github default as of Oct 2020) would remove the potential problems with outdated master, so if you want to rename the main branch this would be a good time to do it. Then tell github to use "main" as the default and delete "master". Github will automatically redirect URLs with master to main. See https://github.com/github/renaming for details.

The warnings still apply: go through build system and docs and replace all references to the old name with the new name. I think there is less room for accidental conflict this way.

wpotrzebowski commented 4 years ago

From https://github.com/github/renaming it seems that it is better to wait until "later" this year as they will provide tools for seamless move to main. What we can potentially do:

Set ESS_GUI to default branch (not remaining to master) and wait until Github provides tools for transition to main
Move master to wx_gui once ESS_GUI to main transition is successful
I think step 1 is relatively safe and can be easily reverted.

RichardHeenan commented 4 years ago

I am presuming that all of us amateur developers will be best (or have noc choice) to start again with a fresh clone of the entire sasview repo once the renaming has happened?

RichardHeenan commented 4 years ago

Just checked out ESS_GUI, but git pull then fails, see below, is it time for me to clone something ? Then tried git checkout main - see more below

git checkout ESS_GUI Switched to branch 'ESS_GUI' Your branch is behind 'origin/ESS_GUI' by 25 commits, and can be fast-forwarded. (use "git pull" to update your local branch) (qt5) CLRC+rkh98@NDW1501 MINGW64 /c/sasview42/sasview (ESS_GUI) $ git pull Your configuration specifies to merge with the ref 'refs/heads/ESS_GUI' from the remote, but no such ref was fetched. (qt5) CLRC+rkh98@NDW1501 MINGW64 /c/sasview42/sasview (ESS_GUI)

$ git checkout main Switched to branch 'main' (qt5) CLRC+rkh98@NDW1501 MINGW64 /c/sasview42/sasview (main) $ git pull There is no tracking information for the current branch. Please specify which branch you want to merge with. See git-pull(1) for details.

git pull <remote> <branch>

If you wish to set tracking information for this branch you can do so with:

git branch --set-upstream-to=origin/<branch> main

(qt5)

Should I do as it says, or will that get me into trouble? Should I just wait a day ? thanks Richard

krzywon commented 4 years ago

@RichardHeenan - the first issue you are having is because the remote branch no longer exists. I would suggest you delete your local ESS_GUI branch.

The second issue is a bit harder to figure. Setting the upstream for main won't break anything on github, but it might effect your local environment. Do you already have a local branch named main? Do you have any uncommitted changes? These both might cause conflicts. You could try to delete your local mainbranch and the check it out again.

If you do these things and are still having issues or don't want to deal with any trial and error, delete the entire directory you have your code in and clone it again. Note - this will wipe out any changes that have not been pushed, committed or otherwise.

RichardHeenan commented 4 years ago

I have not got any code changes to commit, so I think it is indeed time to clone afresh, which is what I suspected we would need to do at some point. Update - fresh clone seems to be working just fine. I cloned sasview, sasmodels, but also docs, and bumps and periodictable. Is it time to update the developer notes at http://trac.sasview.org/wiki/DevNotes/DevGuide/CondaDevSetup5.0

krzywon commented 4 years ago

Good catch @RichardHeenan. I changed the wiki to direct new people to use the main branch for 5.0.

butlerpd commented 3 years ago

I think this issue has now run its course. Main is the default branch and ESS_GUI moved here. @rozyczko, @mdoucet or @wpotrzebowski is there any reason to keep this issue open? or can we close it?

wpotrzebowski commented 3 years ago

@butlerpd I was planning to close when I am done with master -> wx_gui transition, which in principle can be done now as I've disabled all jobs on https://jenkins.esss.dk/sasview/ and we will swap it with https://jenkins.esss.dk/sasview-beta on Jan 25th (at least that's the current plan).

wpotrzebowski commented 3 years ago

The only remaining issue from this list is to rename master to wx_gui branch.

wpotrzebowski commented 3 years ago

master has been copied to wx_gui branch and pushed to github. It should be safe to remove master now?

SasView / sasview

Using ESS_GUI as the main branch #1638