It seems that in the past year or so that Katello developers have been working more and more on the Foreman itself. The Katello team owning the redesign of the All Hosts Page was a big start to it, but we’ve also had developers helping out more with reviews and smaller Foreman issues here and there.
One bottleneck we’ve noticed with working on Foreman PRs is the lack of Katello maintainers who also have merge rights to Foreman. React PRs for Foreman from Katello maintainers might sit for a while waiting on a reviewer who can give a green checkmark. It’s especially true for React PRs, since only a subset of Foreman & Katello developers are comfortable leaving a comprehensive React code review.
While I think part of the solution would be to train more folks on React, I’m proposing that Katello maintainers also get commit access to Foreman, and vice versa. This would be one step short of adding Katello to the Foreman organization, which has been discussed before.
I don’t believe there will be much added risk here since developers tend to know their limits and ask for help if they’re uncomfortable with leaving a green review on a PR. I believe the cross-team-reviewing that would now be possible would help get PRs through faster in the end.
I’m curious what everyone thinks about this. Are there any risks that I’ve missed? Does anyone think there might be a better solution improving cross-team PRs?
Note that the context isn’t verbose, but it’s concise.
Encourage a mutual understanding that even a grey checkmark on a PR is valuable if the user describes what they tested and why they think the PR is ready to merge.
I think the issue of slow reviews is not limited to Katello-originated PRs (and I am also guilty of it). IMHO The solution is to add more maintainers.
I’m not sure if we ever wrote down the process to be added as a maintainer. If we did, I can’t find it and then it’s likely other people can’t either and we need to address that.
In the short term: it’s based on having several commits merged and having doing reviews. Overall, show that you know understand the code base. Then historically we’ve used nominations where an existing maintainer nominates a new maintainer. I’m not sure if everything is properly tagged on Topics tagged nomination, but it’s safe to say we don’t add too many new maintainers while I think it’s health if we do add a few.
Long term, I’d like to describe the governance of the project.
I don’t think we should blanket add everyone from Katello as a Foreman core maintainer, but that we should merge the two organizations. Then treat Katello like other plugins: a team. Members of the Foreman org can already see on Sign in to GitHub · GitHub that every plugin has a team (Foreman landscape shows that too).
In addition to that, some people should be added as core maintainer. But only if they plan to actively participate. Otherwise we’ll have a ton of maintainers but still don’t get reviews.
Absolutely. You don’t need to have merge permissions to submit a review. The checkmark may be grey, but it helps a lot.
Core Foreman also appears to be a bit overwhelmed from my perspective, there are lots of open PRs compared to contributors. I think maintainers of other plugins (namely Katello) could start helping there.
We found this document: Foreman :: Contribute
Some of it might be out of date, but the “rule of 10” that it has felt correct based on how we add Katello committers.
I agree, it’ll help get the two organizations closer together. I’ll take a look sometime into what that might entail.
That’s one downside to blanket access that I didn’t consider, some folks will have the power but rarely use it. Perhaps this is more the time for folks who likely meet the requirements for Foreman / Katello commit access to check and get their nominations if applicable.
Regarding the amount of contributions and reviews over many years in Foreman and Plugins, I think the following folks could/should get commit access so they can help more, we can create follow-up nomination posts for them if there are no concerns.
Thank you for your proposal. The idea of granting Katello maintainers commit access to Foreman, and vice versa, is intriguing and seems like a practical solution to improve the efficiency of PR reviews. Training more team members on React is also a good long-term strategy.
First of all, I’m sorry for a late reply here. I agree that having more committers could help to improve the throughput. I’m getting some hard data on this, but I can already say that there’s a good potential for improvement.
I’d like to point out, that the nominations shouldn’t be based on the quantity of the contributions and reviews but rather their recency, depth and quality. I brought up this idea to the existing core committers and we agreed on it would be best to go through the PR reviewing mentorship first. This is nothing new, we did this with most of the recently added committers too. For some it may be just a formality, for some it make take several sessions. We are in a process of finding the mentors from the existing committers group and they will be reaching out to the folks listed here.
I believe that the mentors will open the nominations as soon as they feel the person is ready.
