Merging PRs in foreman-packaging

The number of open PRs in foreman-packaging is currently ~30. Even though this is a better situation compared to the first half of January when we were at ~40 open PRs, this still seems like a high number to me. Especially if I consider that most of them have ‘Not yet reviewed’ label and have been opened for weeks if not months - some of them are even for stable branches.

This raises a question of how we can improve in this area. I could think of 2 approaches:

A) More people with merge permissions
Having more maintainers with merge access to the foreman-packaging would help to spread the load.
Do you know of anyone you would like to nominate or would you like to be involved in this effort?

B) Automate
Updating plugins seems like a good starting case for automation. We could have a bot that goes though the opened PRs once a day and merge PRs that contain version bump for plugins if the build is green. Of course, there is no limit on conditions, additional ones might be:

  • no dependencies changed
  • merge only when opened against certain branches
  • contributor who opened the PRs is a member of theforeman or katello org

What conditions do you think bot should check before merging?

Do you have an idea about a different approach?

1 Like

I’m always in favor of more automation. I’d love to be able to empower developers to update their dependencies more frequently and efficiently by reducing the time line from PR -> Merged.

Initially, it’d probably be best to limit it to develop branch, and possibly to certain dependencies

Also, we would probably want to limit it to people inside the katello or foreman organizations. It’s not hard to imagine scenarios where accidents or malicious incidents could take place due to us not noticing something slipped under the radar.

As we go along and open it up to stable branches, we probably want to ensure a version update is limited in scope. Some kind of package blacklist may be useful as well.

As far as change conditions, I think ensuring that it’s only version/release and changelog updates would be sufficient.

As of this week, we also now have jobs in our CI that will build packages on merge for release branches. This was previously a small barrier to merging as it required a maintainer to go release things on their command line. This should increase the rate at which plugins get merged into stable branches.

I will also add that Debian packaging is the skillset we lack the most in knowledge wise. If you see Debian PRs sitting, that is likely why and please do ping us.

I am very happy to hear that! this was blocking me from merging prs to stable branches because i was never sure what extra manual actions needed to be done (and did not want to mess up things on koji by taking the wrong action).
Does this mean that all RPM PRs can now be merged with not further manual action? or are there some that still require additional actions (e.g. adding a new package)? If there are, I think even just having an indication on the PR if additional action is needed or not would make me feel much more comfortable in merging prs (and make it easier for others with no koji access to know if they can merge something).

I’m all for starting small. Auto merging version bumps would already go a long way towards @packaging team being able to focus on more interesting prs and make contributors life easier - for simple version bumps just open a pr and once tests pass it will get auto merged.
As we go along this route, we can in the future perhaps find more rules/areas where we can automate this and make it easier.

Another interesting point that we can consider is automatically opening PRs when a new gem version gets released, so we don’t get stuck on older version just because noone bothered bumping the rpm spec.

For stable branch, this is effectively true. If a package is targeted at Foreman or Katello repositories though, for stable branch, they will need to be signed manually before going out. However, they will still build automatically if merged into Koji and the release pipeline will fail indicating the need to sign.

This shouldn’t be too difficult to enable. Once a tag is pushed to the git repo, or a new gem shows up on rubygems, we could begin the update process in an automated fashion

AFAIK obal can’t tag in an existing build. Many packages have the fmX_Y version and are rebuilt for every branch but those that don’t might still fail.