Question: missing patches in develop from 2019.2

[aplanas]

Description of Question

I usually develop against the develop branch, and backport the selected features into my packages, but recently see a lot of problems doing the rebase. The main reason is the rename of several modules in 2019.2 that is not reflected in develop, and one of the commits is this:

33bb5bf#diff-7f7b511c6a61d50c590bcf359b3adbad

I am worried about this, how the divergence of old branch are not reflected in develop, where I expect to find the code that reflect the last decisions and fixes in the project.

I want to ask about the policy for this kind of divergence, and point that this practice makes consume a lot of time tracking changes and backporting / porting fix between branches. Especial note is that is extremely easy to have regressions, as the last fix is not living in develop, so the next branch will contain the bug or the old behavior again.

[Ch3LL]

that commit was recently merged into 2018.3 here: #50812

its jsut making its way to develop via merge forwards. I have this PR open here: #51990 which will include that commit.

[aplanas]

@Ch3LL my bad for not finding the commit in #51990

Due to the size and the fast changing rate of develop (that makes the rebase of this big commit quite a challenge), I guess that will take time to merge.

IMHO a merge forward policy have some disadvantages, like this one, hard to track the missing commits, having a release product with changes that are not available in develop, etc. Is there any mechanism or forum to suggest the revision of the merge forward policy, and evaluate other alternatives?

Thanks!

[Ch3LL]

you can suggest here and i can ping the core team for their opinions.

But I do want to make something more clear when you say "having a release product with changes that are not available in develop," This commit was merged 15 days ago so its not in a release right now, it will be in the next 2018.3 release.

[aplanas]

@Ch3LL forgive me if I am making a mistake here:

But I do want to make something more clear when you say "having a release product with changes that are not available in develop,"

Yes, I am very opaque sometimes.

This commit (#50812) is also living in 2019.2, was merged 19th of Feb 2019 and comes from a commit created 1st of November 2018 (you can see the details here: 33bb5bf#diff-7f7b511c6a61d50c590bcf359b3adbad). So it is not a new commit (Nov 2018), it is present on an already released product (Salt 2019.2), and it is not in the develop branch.

If this commit was reviewed and merged in develop first, the team will send a very clear message about the renaming, and all the commits on top of develop from November, December, January, February, and March (so far) will already obey this renaming (among other things, like more tests), making the job of backporting bits from a new branch a lot more easy.

The commit that you pointed (#51990) shows an important technical debt of what is expected (?) be the most up to date branch of the project, and detecting already taken decisions and fixes (121 commits) in old branches (2017.X, 2018.Y, 2019.Z) makes the contribution a bit more complicate.

[Ch3LL]

it is present on an already released product (Salt 2019.2)

You are right that the commit is from November 2018, but was never merged until 19th of February, so that means its not in a released version, but yea its in the 2019.2 branch, which is a bug fix branch. This commit was actually submitted to fix a bug.

When i run git tag --contains 33bb5bfe69acab4653b9cc7e21857d6b6af31a37 it does not show the 2019.2.0 release. It is only in the 2019.2 branch, which we do release our 2019.2 release from, so that commit will be in the next 2019.2.1 release.

The develop branch is for feature work, and we maintain the other 2018.3 and 2019.2 as bug fix releases. Yes we definitely can merge things into develop and backport them and we do this for some fixes that are bug fixes. In this case it was just merged into 2018.3 and merged forward, and it should be in develop now. If anything we should have just merge forwarded this earlier so as to not cause any confusion, but we were having issues with one of the merge forwards with the tests passing.

But I'll open up the conversation to @saltstack/team-core as well to give their opinions on merge forwards versus merge to develop and backport.

[aplanas]

When i run git tag --contains 33bb5bfe69acab4653b9cc7e21857d6b6af31a37 it does not show the 2019.2.0 release. It is only in the 2019.2 branch, which we do release our 2019.2 release from, so that commit will be in the next 2019.2.1 release.

You are right. The RPM that I can download from the official repository confirm also that this was was not part of a released product. Thanks for clarify that 2019.2 is for the development branch of 2019.2.X, and not the released product. That was my mistake too.

But I'll open up the conversation to @saltstack/team-core as well to give their opinions on merge forwards versus merge to develop and backport.

I am very grateful for this. Thanks!

[waynew]

I'm not sure how strong my opinions are, but one thing that I've noticed while working on some different things in salt:

I tried working off of develop to make a fix for one of the earlier branches. When I did this and tried to backport the changes, there were several issues that I ran into, like entire blocks of code missing, or async being renamed to asynchronous due to async and await becoming Python keywords.

Personally I've found that it's much easier to make the change in the branch and then merge it forward into develop. I do agree that it's more complicated than a typical workflow, but that's part of the price we pay for maintaining older branches... we have to maintain older branches 🤣

It would be a lot nicer if we had a faster way to merge forward changes from other branches into develop, but I'm not sure if there's anything out there that could do that well without manual intervention - unless of course we're happy with develop potentially being entirely broken occasionally. Then we'd just have to setup a hook to merge all the release branches to develop when they've got a merge, and I think that GitHub has that capability.

[aplanas]

Personally I've found that it's much easier to make the change in the branch and then merge it forward into develop. I do agree that it's more complicated than a typical workflow, but that's part of the price we pay for maintaining older branches... we have to maintain older branches rofl

Working in big projects I always find backporting a more natural approach:

• In the master branch you make all the decisions and fixes, this first make a clear statement of the project (critical for communicating with other developers), and this workflow make more guarantees about avoiding regressions.

• Backport from master to maintenance branch can be done after the review in the master branch, so this backport expected to be more fast to merge and easy to review, as the discussion happens before and you are targeting an slower branch (less needs of rebasing, less controversial for merging reviewed fixes)

• Many times (not always) the code in the master branch is a bit more complex, this makes the backport sometime simpler or no-op.

In comparison, merge forward have those problems for me:

• Every commit happens first in the product branch, hiding this fix to the developers, that are usually more focused into the master branch. This can potentially duplicate efforts. Also there is only one branch to monitor to see the current flow of the work.

• Every commit in the product branch is 'new', hiding the fact that sometimes is not a fix but a new feature, in contrast with the other model where any commit to a maintenance branch is always a backport (and cherry-pick -x makes easy to track the commit)

• The master branch can easily miss commits from other branches, as picking fixes from multiple branches is an extra work done manually. The traceability of the change is also more complicated.

It would be a lot nicer if we had a faster way to merge forward changes from other branches into develop,

Can be complicate, as develop moves faster. But backporting from a fast branch to a slow one avoid this problem.

but I'm not sure if there's anything out there that could do that well without manual intervention - unless of course we're happy with develop potentially being entirely broken occasionally.

IMHO is unrelated. Develop being broken is more about a wrong CI / test coverage.

[waynew]

Develop being broken is more about a wrong CI / test coverage.

I was referring to doing automatic merge forwards without regards to any correctness (which, yes, can be tracked by those things), based on the assumption that if develop is moving fast then people should be (re)basing their changes, and would be exposed to the (potentially) broken changes 😊Then humans could fix them.

---

I don't know that I'm firmly convinced that either approach is perfect, but if this is something you feel strongly about, I think it makes sense to codify it in a Salt Enhancement Proposal. That's a more appropriate place for discussion. Would you be interested in creating a proposal to change the policy from merge-forward to merge-backward? Whether or not it the proposal is accepted, I personally think it would be worth the discussion.

[aplanas]

I don't know that I'm firmly convinced that either approach is perfect, but if this is something you feel strongly about, I think it makes sense to codify it in a Salt Enhancement Proposal. That's a more appropriate place for discussion. Would you be interested in creating a proposal to change the policy from merge-forward to merge-backward? Whether or not it the proposal is accepted, I personally think it would be worth the discussion.

I was not aware of this possibility. I will check this out. Thanks for the information!

[aplanas]

@waynew I created this: saltstack/salt-enhancement-proposals#7

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.