[
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)
