But I can’t find information about the maintainer status of this, or if it’s still useful with how we manage code drift and release branches now? Do we still use it? What role/ticket is responsible with reviewing/merging the PRs?
@maxim do we want to update this to add our common ulmo branch of openedx-platform?
cc @Agrendalath as I see you’ve merged some of the sync PRs in the past.
@samuel, we never formalized this process. IIRC, we discussed passing this responsibility to the FFs, but I suppose we forgot to follow up. I used to merge these PRs when I had a moment, but then I had to work on older releases for a while, so I no longer had time and context to test the new ones. Also, resolving some of these merge conflicts became tricky over time, as we accumulated significant code drift.
I suppose it would be easier if we added Ulmo and removed everything else from there. Still, it would be nice to ensure somebody checks these PRs.
Still, it would be nice to ensure somebody checks these PRs.
Agreed! I wonder if this would be worth adding to some particular role. Perhaps to the community liaison, or at least to the community liaison to delegate? I’m easing into that role, and I see all these PRs now as I regularly check and keep tabs on everything.
I feel like to help deal with these kind of PRs, we need:
a quick way to determine which account we can log time to (it will be cumbersome to create a new ticket and discuss accounts for each new dependency update or code sync PR).
a quick way to find someone to delegate addressing it to.
If we have these, it would be easy for someone like the community liaison to eg. ping the FFs with “this Renovate PR just opened, please check and log time to BB-XXX”.
Handling these types of PRs could be considered a form of recurring maintenance, so we could create a recurring ticket for it in the DevOps Maintenance: Recurring Tasks epic and link it to the corresponding account.
Since the Community Liaison will often be the first person to see these PRs, adding the delegation process that you described to the scope of that role would make a lot of sense (and be preferable over introducing a new role just for this purpose)
–
As for the sync PRs, it sounds like we haven’t been merging them of late, at least not with any type of regularity/consistency. If that hasn’t caused any issues (?), we might not need them any longer and could consider disabling the automation that creates them.
If it turns out that we do still need them, it’ll definitely be worth talking some more about how to handle them and make sure they’re not missed going forward.
Maybe we’d want to handle them the same way as Dependabot/Renovate PRs, but since the context (Open edX release upgrades) is a bit different here I’m not sure.
We haven’t been merging them over the last few upgrade. I can’t evaluate whether we need this automation or not without looking deeper into it. For example, I would compare a branch from our fork for one of the previous releases against it’s upstream version (e.g. opencraft-release/sumac vs open-release/sumac) and inspect the commits that are present in the upstream but not in our fork.
I think where it might get tricky is if we want to split maintenance time to different accounts per-repository. For example, Renovate/Dependabot PRs for repos that are specific to a single client probably should come under that client’s maintenance account; and various internal projects are under different accounts (eg. SprintCraft, Listaflow, etc.).
From what I can tell, the intention is to keep our forked release branches up to date with new commits to the upstream release branches. This feels like a good thing to do, to reduce code drift and ensure we’re sticking close to upstream. Especially now since we’re moving toward using release branches for openedx-platform (eg. release/ulmo) rather than more static tags (eg. release/ulmo.1).
I guess the risk is that if we don’t have this kind of automation, then our release branches will fall behind and miss out on any backports/bugfixes from upstream.
Starting with just the main ticket under SE-6125 / Instances DevOps and using that for Renovate/Dependabot PRs against any repos that don’t contain/define any code/infrastructure that’s specific to a client or internal project.
Creating subtasks and linking them to client/project-specific accounts as necessary – i.e., if and when the first individual PR comes in that is specific to a client or internal project.
Putting a note in the description of the main ticket to mention that time should be logged on appropriate subtasks.
You’ll only need to create these subtasks once, and the full set will build over time.
(If you want to create a ticket for doing the initial setup and updating the Community Liaison role definition in the handbook, feel free to put me as reviewer.)
–
Re: sync PRs
@samuel@maxim I see where you’re coming from and I agree, keeping common/shared release branches up-to-date with backports and bug fixes from corresponding upstream branches seems like a good idea in principle. I.e., in the sense that it would make it easier for client owners to update their clients’ instances with those upstream improvements between upgrades.
How frequently client owners would actually do that, I’m not sure. For more mature instances, the number of redeployment opportunities tends to go down. Unless there’s a business case to be made (e.g. because a bug has been affecting users of a client instance directly, or a client needs some backported functionality to become available to their users as soon as possible), clients won’t necessarily want or need their instances to be redeployed just because a release branch got updated with additional changes.
Client owners would also need to track what’s happening in common/shared branches to know about bug fixes and backports being added to them in the first place.
So for the practice of tracking and merging sync PRs to be useful, it seems like there’s more that needs to happen than just the practice itself. And it would be worth making sure we’re not spending time/effort/budget on it just for the sake of keeping branches in sync
I see two options for moving forward here:
Using past data to get an idea of what we missed by not merging the sync PRs, as you suggested @maxim:
And/Or:
Picking the practice back up again (with some modification to keep client owners in the loop) and evaluating its usefulness at some point in the future; 6-9 months from now maybe.
For the second option, we could consider integrating it into the process for Renovate/Dependabot PRs discussed above: Time spent merging sync PRs could be logged on a subtask of the main ticket under SE-6125, which we’d link to the Instances Upgrades account.
