search

CCI-MOC / m2

Bare Metal Imaging (Malleable Metal as a Service)
18 stars 16 forks source link

Development is not happening in the master branch #102

Closed knikolla closed 7 years ago

knikolla commented 7 years ago

PRs are being made and merged against the dev branch instead of master. The master branch should be where all the development is happening.

chemistry-sourabh commented 7 years ago

@knikolla Master branch should always be the stable branch right ? Not the latest

knikolla commented 7 years ago

@chemistry-sourabh master should be the latest. Here's a brief description of how this model works.

Let's say you release a version codenamed hero. This release maps to your 0.2 release. You would create a new branch named hero from master (which is your latest development branch), and tag the current latest commit in the hero branch as 0.2. This is your stable branch for this specific release. You can have many stable branches for many releases, depending on your EOL support policy.

Let's say you are working in master towards the 0.3 release, but it's not quite ready yet. 0.3 contains non-backwards compatible changes, so you can't release it as 0.2.1. However a very important security bug was just discovered and you need to "backport" the fix for it to 0.2.1, because that release is still stable and supported. What happens is that you fix the bug in master, and cherry pick that specific commit into the hero branch, then tag the current latest commit in hero as 0.2.1. (Repeat for all supported branches).

Please note, I'm not opposed to development branches for specific features. However that branch must be short-lived and specific to that single feature and must have fully merged back to master and the branch be deleted for it to be released.

chemistry-sourabh commented 7 years ago

@knikolla We were sort of aiming for the gitflow workflow https://www.atlassian.com/git/tutorials/comparing-workflows#gitflow-workflow

chemistry-sourabh commented 7 years ago

Also @knikolla in your approach when will we merge the release branch into master ?

knikolla commented 7 years ago

@chemistry-sourabh The release branch is never merged back. Why would you need to merge it back into master?

naved001 commented 7 years ago

AFAIK, the reason we decided on a dev branch was because, the current master is dead and the code in production was coming from @chemistry-sourabh's personal github fork. I wanted the code to be a part of CCI-MOC. But then we couldn't directly merge it to the master here without fixing some really important issues, which is why we made this dev branch. We have some issues tagged as high priority, once we have those solved we will make this dev branch the master branch and then follow the standard git workdlow. @apoorvemohan you want to add something to it?

apoorvemohan commented 7 years ago

The current master branch is dead. The 0.5-release will be made the master branch. It will be released soon.

chemistry-sourabh commented 7 years ago

@naved001 there is no standard git workflow, it is what your organization decides on. I agree that the current dev will be master (or merged into it). @apoorvemohan will we have another dev branch or not ?

apoorvemohan commented 7 years ago

yes we will have a dev branch

knikolla commented 7 years ago

@chemistry-sourabh @apoorvemohan for all purposes, a long lived dev branch is the same as doing all development on master. So let me know what you think the advantages of having a dev branch are.

chemistry-sourabh commented 7 years ago

@knikolla why I want to merge to master is to have a single source of truth. As per my understanding If a new release is made (Lets say 0.3) it will be branched from 0.2 right ?

knikolla commented 7 years ago

@chemistry-sourabh The single source of truth is master. Releases are branched from master. 0.2 and 0.3 are both branched from master.

chemistry-sourabh commented 7 years ago

@knikolla Ok but since we are not merging 0.2 back to master then 0.3 wont have 0.2 changes right ?

knikolla commented 7 years ago

@chemistry-sourabh Fixes to 0.2 are cherry picked from master.

chemistry-sourabh commented 7 years ago

@knikolla Now I am confused, it probably is I am missing something about git.

The team developed 0.2 and released it (But didnt merge it to master). Then they started to work for 0.3 which is based from master. Now master wont have 0.2 changes right ?

knikolla commented 7 years ago

@chemistry-sourabh you are thinking about it wrong.

The team developed master, and decided to release 0.2. 0.2 is released from master and becomes a new branch. At that exact moment in time master == 0.2.

The team starts work on 0.3, this work is done in master. Master includes now all changes from 0.2 and extra changes for 0.3. Team decides to release 0.3, they branch from master again. Now 0.3, includes everything in 0.2 (unless something was specifically added to the 0.2 branch without being cherry picked from master).

apoorvemohan commented 7 years ago

@knikolla I'll discuss this with you tomorrow when I come to MOC.

chemistry-sourabh commented 7 years ago

@knikolla I think I got it now. The branch is made only at the time of release until then all commits are made to master?

I feel that your approach and our approach are inverses of each other. We wanted the master branch kind of like a release branch which people just clone and start using. We will be using tags to denote the release on the master branch.

Anyways @apoorvemohan will discuss with you tommorow. During a release, I just want a branch for bug fix PRs related for the release and a dev/whatever branch where everything else goes, I dont have an issue with whatever gives me that.

knikolla commented 7 years ago

@chemistry-sourabh they are not inverses. They are actually almost the same with only a few differences. Think about dev in your approach as master in mine. And think of master in your approach, as specific (multiple) release branches in mine. This allows you to have multiple supported releases at a single point in time. In this case, instead of merging dev into master, you branch the release out of master.

The only difference is that I argue against bug fix PRs to be made in release branches, and instead to be made in the branch where all development happens and cherry picked to release branches. This is known as backporting.

chemistry-sourabh commented 7 years ago

@knikolla the bug fix branch will be based off from the dev branch and will be merged in master when done. It will also be merged in dev to get the changes.

I wanted a bug fix branch so that other dev who are working on other features don't get blocked.

knikolla commented 7 years ago

@chemistry-sourabh you can't merge the entire branch because you will get the entire commit change history from the development branch to the release branch.

chemistry-sourabh commented 7 years ago

@knikolla Ok squash and cherry pick

knikolla commented 7 years ago

@chemistry-sourabh do you see now why you don't really need a bugfix branch in CCI-MOC/ims? You'll have one in your fork from which you'll do the PR, which will be sqash-merged into master and cherry picked.

chemistry-sourabh commented 7 years ago

Also thinking about it merge also should be fine as dev and master will be in sync except for the delta that is the release ?

knikolla commented 7 years ago

@chemistry-sourabh release shouldn't be ahead of master/dev.

chemistry-sourabh commented 7 years ago

@knikolla the bug fixes will be merged to master and dev right ? I thought the other branches can stay ahead until merged.

knikolla commented 7 years ago

@chemistry-sourabh i meant the branch pertaining to a release. feature branches can be ahead.

chemistry-sourabh commented 7 years ago

@knikolla During the release time you are saying that all bug fixes will go to master/dev right ? What about the other devs who are working on some other feature ?

knikolla commented 7 years ago

@chemistry-sourabh Why should they be impacted by bug fixes?

chemistry-sourabh commented 7 years ago

@knikolla We cant merge the other PRs into master/dev until the release is done and a release branch is completed. So those PRs will be hanging until the release is out.

knikolla commented 7 years ago

@chemistry-sourabh what do you mean by a release branch is completed?

chemistry-sourabh commented 7 years ago

@knikolla sorry created

knikolla commented 7 years ago

@chemistry-sourabh So let's go through the 2 possibilities.

  1. It's very early in the release cycle (pre-alpha), and you can merge new features for this upcoming release. Bug fixes don't impact the work of other developers, both can go into the main development branch.
  2. It's late in the release cycle (beta), and you can't merge new features this late. Only bug fixes are allowed. In this scenario you should have really already created a release branch (prior to releasing it as stable), and optionally tagged the beta release. Bug fixes can go into the main development branch and get backported to the release branch, new features can go into the main development branch and not compromise the release.
chemistry-sourabh commented 7 years ago

@knikolla ok this looks fine. Another question what if we get a PR for a future release, Like 0.2 is going on, but we get a PR which wont be part of this release (For > 0.2), what then ?

knikolla commented 7 years ago

@chemistry-sourabh if it's ready to be merged, you should probably merge it and release it.

chemistry-sourabh commented 7 years ago

@knikolla @apoorvemohan wont agree with that lol

knikolla commented 7 years ago

@chemistry-sourabh I'll talk with @apoorvemohan tomorrow.

ravisantoshgudimetla commented 7 years ago

Not sure if everyone got the answers they need.

@knikolla - So, as of now master branch has something working, we are trying to make some changes to codebase that are disruptive. True that we could have made "releases" and explicitly put a sentence in documentation saying that master branch could be unstable because of development and please use release0.2,0.3 etc. But, we chose this approach and hopefully we can change from next releases.

Just to make sure to make sure that everyone understands, this is not a hard rule. Usually, dev branch(this could be used for nightly builds) could be merged to master once it is feature rich and fully functional and developers can branch off "dev" branch. I have seen some orgs using 4-5 branches like hot fixes, bugs etc. But what @knikolla suggested is more than enough for our codebase .

@chemistry-sourabh

ok this looks fine. Another question what if we get a PR for a future release, Like 0.2 is going on, but we get a PR which wont be part of this release (For > 0.2), what then ?

Usually it depends on workflow we choose. But in general

Into master branch: If its not critiical, let it hang in there till the release completes during code freeze. If there are bug fixes and some stabilizing changes they can go in. If there is no code freeze it can be merged.

Into dev branch: Different orgs have diff perspectives, usually dev branch will be unstable branch, where developers can break anything at any point of time as all the features are going in. There won't be a code-freeze for this as long the merge to master is stopped.

sirushtim commented 7 years ago

I agree that we don't need a dev branch either for all the reasons mentioned above. Also, PR's are effectively a dev branch and github has a better interface to deal with PR's than branches. We can just categorize different PR's to be either a feature, bug or a hotfix by adding labels.

Furthermore, I believe we should start using Semver http://semver.org/ for releases. Semver contains rules to consistently bump version numbers in releases. Mostly, it has a set of guidelines to restrict what can be merged during and after a release. This will allow us to define and also consumers of BMI to know if any backwards in-compatible changes are made.

ok this looks fine. Another question what if we get a PR for a future release, Like 0.2 is going on, but we get a PR which wont be part of this release (For > 0.2), what then ?

I'm against merging code that is not in a finished state since usually the cleanup of the aftermath is de-prioritized and also counter-productive. If it is for the purpose of a demo, one can just patch BMI separately in ones own branch and use it, it doesn't have to be necessarily merged though.

naved001 commented 7 years ago

With #143 we have everything in master now and that's where development will happen now.