[jira] [Comment Edited] (FINERACT-1154) Git branch strategy is wrong, use tags instead

["Michael Vorburger (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FINERACT-1154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17207581#comment-17207581 ] 

Michael Vorburger edited comment on FINERACT-1154 at 10/4/20, 3:50 PM:
-----------------------------------------------------------------------

Re. the "strategy", I think we may be overthinking this? ;) To me, by the time we create a release branch (e.g. for upcoming 1.5.0), last minute blockers that still need fixing on that release branch should be rare - hopefully. So manually cherry picking a select few last changes from the develop to the release branch seems appropriate. Just the way how we did it for 1.4.0 seemed about right, at least to me.

We exceptionally could make a choice to just rebase a release branch on develop. [~aleks] didn't you actually consider this for 1.4.0 at some point? I have a vague memory that this came up on... maybe on some PR, and I probably had suggested that we should not? If we do do decide to do this for a release, I'm less worried about the history rewrite and required git push --force for a release branch (but not develop, see below), and more about that this basically means we branched too early. This to me is basically the git equivalent similar to saying "oups, we ran too fast, let's just ditch the previous release branch cut, and start over and grab the latest and greatest from develop, after all".  Doing this after the respective email to the list essentially seems like a process failure, to me - why didn't more people speak up on list earlier, to delay the release?

We however IMHO can (should) never rebase develop on a release branch, and force push. That seems fundamentally wrong, to me. Actually, if the ASF has properly configured GitHub, they should have made develop a "protected branch" to which we cannot force push (but I don't know if they do that; perhaps e.g. [~gmcdonald] would like to chime in about that?).

Re. "the release tag never appears in the commit history on develop", that actually seems "normal", to me. Unless we do not add cherry-pick any last minute fixes from develop to a release branch, this is "by design", and expected, isn't it? Do you think it's a problem?

Re. "should we not rebase develop on the latest release tag after release", I would say generally No, we shouldn't have to, if we (RM) do everything right. But re. "be sure that all the fixes applied in the release branch are also included in develop", to "merge the release branch back to develop"  actually could be a useful step... BUT, the way I look at it, if the Release Manger did a "perfect job", shouldn't that be "empty"? I've just really briefly tried this out, and it almost worked, but showed just a few conflicts - they seemed pretty minor, to me. [~aleks] if you want, perhaps would like to try that for yourself and see if the conflicts that shows are a few minor things that should be fixed on develop, so that such a "release branch merge" comes up "empty". Aleks, re. your point about "we would see in git log two separate commits for the same features/fixes", isn't that only if they are not exactly the same? Perhaps my Git foo isn't 100% accurate here... but I've only noticed a bunch of very minor conflicts in {{.travis.yml}}, {{README.md}}, {{docs/developers/swagger/client.md}}, {{docs/developers/swagger/client.md}}, {{fineract-provider/build.gradle}}, {{fineract-provider/config/swagger/templates/gradle-wrapper.properties.mustache}}. -- If you guys want to make sure that we do don't forget to do that in 1.5.0, do you want to document it on that Wiki page?

BTW I'm not sure what "cherry pick the features/fixes from develop instead of creating new PRs" means.. I do firmly believe that ALL changes to ALL branches, develop or release, should ALWAYS go through a PR. It's too easy to break the build if you just locally cherry pick and push straight to the release branch. But yes any "port" from develop to a release branch should be a cherry-pick (and then a PR for it) - isn't that exactly what we did for 1.4.0? Does it need to be more clearly documented? It seems obvious, to me, but I'm all for making it more clear, if we can.

> The only one I wasn't sure what to do with was the "never released" 1.3.1 branch. I've left that for now, but for sake of cleanliness it might make sense to delete that as well?

Re. 1.3.1, given that we now closed FINERACT-877, I think also deleting that branch is entirely appropriate; I've just done so, see comment in FINERACT-877.

PS: Perhaps an email to the dev list to share that we agreed here to not keep release branches open may be appropriate.. I could send that, unless you would like to - but would keep it really brief and succinct.


was (Author: vorburger):
As per discussion over in FINERACT-1154, I've also deleted our 1.3.1 branch, and created a {{1.3.1-unreleased}} tag for it, just on the (very unlikely) off change that someone would ever like to resurrect it and actually formally drive a release for it; here's exactly what I did just for the record:

{{git checkout 1.3.1
git tag 1.3.1-unreleased -a -m "see FINERACT-877"
git checkout develop
git push origin 1.3.1-unreleased
git branch -d 1.3.1
git push origin :1.3.1}}


______


Re. the "strategy", I think we may be overthinking this? ;) To me, by the time we create a release branch (e.g. for upcoming 1.5.0), last minute blockers that still need fixing on that release branch should be rare - hopefully. So manually cherry picking a select few last changes from the develop to the release branch seems appropriate. Just the way how we did it for 1.4.0 seemed about right, at least to me.

We exceptionally could make a choice to just rebase a release branch on develop. [~aleks] didn't you actually consider this for 1.4.0 at some point? I have a vague memory that this came up on... maybe on some PR, and I probably had suggested that we should not? If we do do decide to do this for a release, I'm less worried about the history rewrite and required git push --force for a release branch (but not develop, see below), and more about that this basically means we branched too early. This to me is basically the git equivalent similar to saying "oups, we ran too fast, let's just ditch the previous release branch cut, and start over and grab the latest and greatest from develop, after all".  Doing this after the respective email to the list essentially seems like a process failure, to me - why didn't more people speak up on list earlier, to delay the release?

We however IMHO can (should) never rebase develop on a release branch, and force push. That seems fundamentally wrong, to me. Actually, if the ASF has properly configured GitHub, they should have made develop a "protected branch" to which we cannot force push (but I don't know if they do that; perhaps e.g. [~gmcdonald] would like to chime in about that?).

Re. "the release tag never appears in the commit history on develop", that actually seems "normal", to me. Unless we do not add cherry-pick any last minute fixes from develop to a release branch, this is "by design", and expected, isn't it? Do you think it's a problem?

Re. "should we not rebase develop on the latest release tag after release", I would say generally No, we shouldn't have to, if we (RM) do everything right. But re. "be sure that all the fixes applied in the release branch are also included in develop", to "merge the release branch back to develop"  actually could be a useful step... BUT, the way I look at it, if the Release Manger did a "perfect job", shouldn't that be "empty"? I've just really briefly tried this out, and it almost worked, but showed just a few conflicts - they seemed pretty minor, to me. [~aleks] if you want, perhaps would like to try that for yourself and see if the conflicts that shows are a few minor things that should be fixed on develop, so that such a "release branch merge" comes up "empty". Aleks, re. your point about "we would see in git log two separate commits for the same features/fixes", isn't that only if they are not exactly the same? Perhaps my Git foo isn't 100% accurate here... but I've only noticed a bunch of very minor conflicts in {{.travis.yml}}, {{README.md}}, {{docs/developers/swagger/client.md}}, {{docs/developers/swagger/client.md}}, {{fineract-provider/build.gradle}}, {{fineract-provider/config/swagger/templates/gradle-wrapper.properties.mustache}}. -- If you guys want to make sure that we do don't forget to do that in 1.5.0, do you want to document it on that Wiki page?

BTW I'm not sure what "cherry pick the features/fixes from develop instead of creating new PRs" means.. I do firmly believe that ALL changes to ALL branches, develop or release, should ALWAYS go through a PR. It's too easy to break the build if you just locally cherry pick and push straight to the release branch. But yes any "port" from develop to a release branch should be a cherry-pick (and then a PR for it) - isn't that exactly what we did for 1.4.0? Does it need to be more clearly documented? It seems obvious, to me, but I'm all for making it more clear, if we can.

> The only one I wasn't sure what to do with was the "never released" 1.3.1 branch. I've left that for now, but for sake of cleanliness it might make sense to delete that as well?

Re. 1.3.1, given that we now closed FINERACT-877, I think also deleting that branch is entirely appropriate; I've just done so, see comment in FINERACT-877.

PS: Perhaps an email to the dev list to share that we agreed here to not keep release branches open may be appropriate.. I could send that, unless you would like to - but would keep it really brief and succinct.

> Git branch strategy is wrong, use tags instead
> ----------------------------------------------
>
>                 Key: FINERACT-1154
>                 URL: https://issues.apache.org/jira/browse/FINERACT-1154
>             Project: Apache Fineract
>          Issue Type: Bug
>            Reporter: Michael Vorburger
>            Assignee: Petri Tuomola
>            Priority: Major
>             Fix For: 1.5.0
>
>
> It seems wrong to me that we have 20 open branches (on https://github.com/apache/fineract/branches), including for the just released 1.4.0. IMHO a 1.4.0 should be a tag not a branch, and there could be a branch named 1.4.x instead - if anyone actually wanted to maintain that (which I doubt anyone does).
> [~aleks], [~ptuomola] or anyone else reading along here, do you agree?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)