Skip to content

Rethink our branching model and workflow #3612

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

Closed
seisman opened this issue Jul 6, 2020 · 33 comments
Closed

Rethink our branching model and workflow #3612

seisman opened this issue Jul 6, 2020 · 33 comments

Comments

@seisman
Copy link
Member

seisman commented Jul 6, 2020

Problem

Currently, the GMT repository has two long-lived branches, master and 6.1. The master branch is unstable and developing, which hosts the codes for the next minor release (e.g., 6.2.0), and the 6.1 branch is for maintaining the GMT 6.1.x. All bug-fixes commits should be merged into 6.1 branch first, and then merge the 6.1 branch into the master branch. All new features should be merged into the master branch only.

It usually causes problems for new contributors and even core developers, since master is the default branch. Contributors may create a bug-fix PR based on the master branch, but the bug-fix PR should be merged into 6.1 branch instead.

Below are some discussions in #3610.


Due to the design of our branching model (master and 6.1), I can see that other new contributors are also likely choose the wrong base branch, since master is always the default one.

Originally posted by @seisman in #3610 (comment)


Others and me. It was happening all the time when we had a 6.0 branch

Originally posted by @joa-quim in #3610 (comment)


We may make less mistakes if we develop GMT 6.1.x in the master branch and add new features to 6.2 branch.

Originally posted by @seisman in #3610 (comment)


We could probably determine the statistics on this, but from gut feeling the vast majority of commits are in response to fixing a problem. So currently, that means every time we have to remember to switch the branch to 6.1. Since I have 6.1 checked out when I make a PR, it is always surprising that GitHub opens up a PR to be merged with master instead of the branch I am in. So while we will screw up from time to time with any of these schemes, perhaps less screwups if we made a 6.2 for new things and continued our habit of doing most stuff to master. That matches the pattern we create a new branch to test new features anyway.

Originally posted by @PaulWessel in #3610 (comment)


Since I have 6.1 checked out when I make a PR, it is always surprising that GitHub opens up a PR to be merged with master instead of the branch I am in.

It's possible to set the default branch to "6.1", but we have to remember to update the default branch when we release GMT 6.2.0 and create the 6.2 branch.

image

Originally posted by @seisman in #3610 (comment)

@PaulWessel
Copy link
Member

Seems like remembering to update the default branch is easily done by adding it to the release check list.

@seisman
Copy link
Member Author

seisman commented Jul 6, 2020

Seems like remembering to update the default branch is easily done by adding it to the release check list.

If I were a new GMT contributor and I want to submit a PR to fix a bug (most new contributors start to contribute to a project by submitting PRs fixing typos in documentations), most likely I will create a bug-fix branch from the master branch, make some edits and open a PR. Even though we set the default branch to 6.1, the PR still contains some commits from master branch.

@seisman
Copy link
Member Author

seisman commented Jul 6, 2020

Ping @GenericMappingTools/python for some inputs, since PyGMT may also face the same problem in the future.

@joa-quim
Copy link
Member

joa-quim commented Jul 6, 2020

I don't think there is a unique solution to this.

  1. Agree that most commits are bug fixes, but after a while thy will spread into fixes in 6.1 and fixes in 6.2 so this won't help much as a distinguishing factor.
  2. When one clicks on the Edit on GitHub button we land on (I assume) default branch. Currently master.
  3. I still like to think in master as the place where development should go but @seisman is right that a commit from a contributor will very likely come from the master branch, and we don't want it to go into 6.1 when it starts to contain new features.

Options

  1. Go with @seisman proposal
  2. Keep going and deal with mistakes
  3. Make 6.1(.1) short-lived so that we won't have time to fall deeply in this dichotomy.
  4. The hard and correct way. Everything goes into master but bug-fixes are picked and applied to 6.1. Cool but have no idea on how to do this and if I learn and find that it's very boring I'll stop with minor bug fixes.

Notice that we followed option 3 in 6.0, but it was not after a short-lived period.

@seisman
Copy link
Member Author

seisman commented Jul 6, 2020

  1. When one clicks on the Edit on GitHub button we land on (I assume) default branch. Currently master.

Not exactly. The button for the latest documentation links to the 6.1 branch, and the button for the dev documentation links to the master brach. We can change the links if we want.

  1. The hard and correct way. Everything goes into master but bug-fixes are picked and applied to 6.1. Cool but have no idea on how to do this and if I learn and find that it's very boring I'll stop with minor bug fixes.

I agree it's the correct way. To do this, you need to know the commit hash, checkout the 6.1 branch, run git cherry-pick commit-hash to apply the commit to 6.1 branch. I believe it can't be done in the GitHub UI, and it's definitely easy forgetting to do the cherry-pick operation.

@weiji14
Copy link
Member

weiji14 commented Jul 6, 2020

Crossref GenericMappingTools/pygmt#424.

  1. The hard and correct way. Everything goes into master but bug-fixes are picked and applied to 6.1. Cool but have no idea on how to do this and if I learn and find that it's very boring I'll stop with minor bug fixes.

I agree it's the correct way. To do this, you need to know the commit hash, checkout the 6.1 branch, run git cherry-pick commit-hash to apply the commit to 6.1 branch. I believe it can't be done in the GitHub UI, and it's definitely easy forgetting to do the cherry-pick operation.

Definitely don't want to discourage bug fixes just because it's a chore. If only there was a way to label a PR as 'backport' and have a CI automatically cherry-pick it to the 6.1 branch.

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

I wanted to write this before my most recent mistake (#3622).

Let just make our lives simpler by facing the fact. We will never release a 6.1.1 Why? We never did in the past. Yes, GMT5 had several point releases but they were either because in the SVN times we had years living branches, or because during the 6.0 development we released a couple of very similar 5.4.x versions.

In more recent times we maintained a 6.0.1 till we got fed up with the mistakes and realizing that it wouldn't make sense to release a point release and leaving out all the work that was already in master. What I'm saying is that the exact same think will happen with 6.1. Paul will resume the 3D interpolations, I want to merge the shakemap branch (had request for it), other developments will soon show up, and within a month we will be in the exact same situation as with 6.0.1

So, let just drop 6.1 and go on with master only.

@seisman seisman pinned this issue Jul 7, 2020
@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

We may find a few critical bugs in GMT 6.1.0 or breaking changes of other GMT dependencies (e.g., GDAL 3.1.1, netCDF 4.7.4) in the next few weeks. Then we have to make a patch (if we can) and release a new version. At that time, the master branch may already contain some new features, then we have to release 6.2.0, even though we just released GMT 6.1.0 a few weeks ago. I don't think it's ideal.

@PaulWessel
Copy link
Member

I think the compromise is this: When we release something like 6.x.0 we suddenly get more scrutiny of this version, and it is quite common that some dumb bugs are found pretty quickly. We already had the guy who could not compile because of a bug in an #ifdef branch for one of the alternative PCRE libraries we say we support - but that none of us obviously are using. So I think the first month or so after a release is a window were we need to be careful and assume there will be a 6.x.1. I don't thing there will be 6.1.2, 6.1.3 etc for the reasons @joa-quim mention. But 6.1.1? Pretty much guaranteed now I think. We might as well plan it for Aug 1.

@PaulWessel
Copy link
Member

One more point: We will be running a short course with 150 participants. Would not be surprised if we discover some more problems that will warrant a 6.1.1 right afterwards.

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

One thing is certain. We should NOT jump the release candidate stage. It would have caught us the #ifdef and the Julia breaking.

@PaulWessel
Copy link
Member

yes, true. All because of stress and too much work leading up to the fixed schedule of the workshop. It was terribly rushed and I worked very long hours with lots of PR being developed in parallel - a recipe for bugs.

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

We should NOT jump the release candidate stage. It would have caught us the #ifdef and the Julia breaking.

I don't think a release candidate does help. Candidate releases can't go into homebrew, macports or any Linux distros. Then there are only very few users using these candidate versions.

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

Maybe true, but I don't think the #ifdef case came from an official package release and don't know the origin of the Julia breaking finding.

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

For most users, building gmt is a tough task. Unless we have tens of volunteers who are willing to build and run candidate releases to help us do more extensive testing, candidate releases won't help us much.

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

One more point against candidate releases is that it takes time to make a release, we perhaps don't want to make a few candidate releases and a final release in one or two weeks. Of course it's doable if we can have a list of volunteers to help test candidate releases and can make the release more automatic.

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

True release candidates turn into final releases without ANY further change if no problems are reported. By ANY it is meant that the tar balls and installers are just renamed.

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

There are always GMT bug reports in the first few weeks of a release. 🤦

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

By ANY it is meant that the tar balls and installers are just renamed.

No, GMT version is hard-coded in the source code. It's confusing to install 6.2.0 but gmt --version gives 6.2.0rc1

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

Nothing stops us to hard-code the GMT version 6.2.0 and call the files 6.2.0rcx and later just rename them.

Anyway, I find the parallel versions situation unsustainable for me.

@PaulWessel
Copy link
Member

Unsustainable? I don't understand - it is not that hard to remember to fix bugs in 6.1 and add new feature to master?

@joa-quim
Copy link
Member

joa-quim commented Jul 7, 2020

That is easy, now being in the right branch when committing and land the commit in the right branch without forgetting ... it just 3 or 4 attempts right now and ended committing directly from the github page.

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

I agree with @joa-quim that parallel versions cause more problems.

it is not that hard to remember to fix bugs in 6.1 and add new feature to master?

It's not that hard for core developers, but we want to attract more GMT users to contribute (e.g., typos, documentation, tiny fixes or even new features) to make GMT a community-driven and sustainable project. Apparently, maintaining parallel versions causes some trouble for @anbj and even @joa-quim.

One more thing is that "merge 6.1 into master" creates so many empty merge commits, which also makes it hard to track and understand the git history. For examples, currently the master branch is 11 commits ahead the 6.1 branch, while 10 commits are the useless merge commits.

After more thinking, it seems the idea of candidate releases are more appealing. What we can do is:

  1. Have a list of users who volunteer to help us test candidate releases
  2. Make a RC release two weeks before the scheduled release date
  3. Notify all volunteers that we have made a RC release and hopefully they can install and test it in two weeks
  4. Fix bugs based on the feedbacks and make the final release on the scheduled date
  5. Still create the 6.2 branch. If there are any critical bugs, then fix it in 6.2 and release 6.2.1 ASAP. For any non-critical typos or bugs, commit them to master, not into 6.2 branch. @PaulWessel may be the person to decide if a bug is critical.
  6. Developing on the master branch, and prepare for the next 6.3.0 release

@PaulWessel
Copy link
Member

For the record, I will be fine with whatever scheme you all are comfortable with. I do not mean to impose any draconian system on anyone. I totally understand the point of making it simple for new volunteers and developers.

@seisman
Copy link
Member Author

seisman commented Jul 8, 2020

@joa-quim Please see my comment above works for you.

@joa-quim
Copy link
Member

joa-quim commented Jul 8, 2020

Yes, with two comments

  • 2 weeks is too long (one week should be good)
  • Drop the voluntary list. Voluntaries, if they are it, will show up without lists

@seisman
Copy link
Member Author

seisman commented Jul 8, 2020

  • 2 weeks is too long (one week should be good)

The time is flexible.

  • Drop the voluntary list. Voluntaries, if they are it, will show up without lists

The reason for the voluntary list is that, we can ping them so that they know we have made a RC release. Otherwise, they may miss the notification unless they watch the github release, visit the website or forum every few days. The voluntary list also gives them credits for helping making GMT better.

@KristofKoch
Copy link
Contributor

Gentlemen of the @GenericMappingTools/core team,

following your discussion above I'd like to give my view as a minor contributor to GMT: It is my first and only contact with the git/github universe. I find the git workflow rather daunting in general. Every time I try to contribute I meticulously follow Aaron Meuer's tutorial and hope not to screw anything up.

So for my kind of contributions (typos or simple editing tasks) with my kind of background (no diploma in git/github usage) I would second @seisman's concerns from above that it would be another cliff a new(-ish) contributor has to circumnavigate which might hold him off from contributing.

Please understand this post as an input to the discussion from a small volunteer without the enormous background you guys have.

@seisman
Copy link
Member Author

seisman commented Jul 9, 2020

I think both @joa-quim and @PaulWessel agree that we will commit into the master branch (developing branch for the 6.2.0 release), and only backport critical fixes into the 6.1 branch (for the 6.1.1 release). It not only make the maintenance easier and also more friendly to contributors.

For backport, mergify is a great tool to automatically create PRs for backporting. However, the backport feature is only free to repositories <= 500 Mb (GMT is ~700 Mb). So we can't use it. There are a few other easy options: https://github.com/marketplace?type=actions&query=backport. They may work or not. Before we setup a automatic way to backport fixes, I'll backport fixes manually.

@seisman
Copy link
Member Author

seisman commented Jul 11, 2020

We already adopt the new branching model and workflow, and also have a bot doing the backports automatically (#3636). The backport bot works well so far.

What about the idea of a voluntary list? As explained above,

The reason for the voluntary list is that, we can ping them so that they know we have made a RC release. Otherwise, they may miss the notification unless they watch the github release, visit the website or forum every few days. The voluntary list also gives them credits for helping making GMT better.

What I want to do is creating a voluntary list in the wiki page. The wiki page explains how the testing program works:

  1. we release a rc version
  2. notify volunteers by pinging their github user accounts
  3. if they're still interested and have time, they can install the rc release and report bugs to us in one or two weeks before the final release

The wiki page is public and everyone can edit it, so that they can add or even remove themselves from the list. The list contains some information, e.g., github username, operation system.

@joa-quim
Copy link
Member

I'm not very much fun of the list idea (had a post that got lost possibly because I missed hitting the comment button) but ok.

@seisman seisman unpinned this issue Jul 15, 2020
@leouieda
Copy link
Member

leouieda commented Jul 16, 2020

Why not use the forum for that? When there is an RC, post to the forum announcement requesting people to test and post feedback there. People submitting feedback can be added as testers to the release notes.

@seisman
Copy link
Member Author

seisman commented Jul 31, 2020

Using forum is OK to me. Closing the issue.

@seisman seisman closed this as completed Jul 31, 2020
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

6 participants