A while ago I proposed code owners for the packaging files. We tried this[1] but didn't work in our setup. It turns out that those mentioned as owners need permissions on the repository. In our case the packaging team (not individual members) has no permission.
There are 3 options:
1) Remove the file since it doesn't work
2) Create a packaging-core group with the intersection of both groups
3) Give @theforeman/packaging permissions on core
Personally I'd really like us to work together better and think this could reduce our packaging issues but have no preference between 2 or 3.
This could use some opinions. Right now weāre missing a notification system for packagers that these changed. A 4th option is to build some sort of notification system ourselves but Iād like to avoid that.
Ultimately I trust our packagers, since they control what goes to the users - and in any case, we can revert changes if someone does something bad, itās all in the git history , so (3) it is!
Iāll put in the obilgatory comment that this is a paid-for commercial service, with a free tier, so factor in considerations of what happens if they decide to remove the free tier later. Also note their terms of service explicitly permit sharing data with third-party organisations. Iām not a fan of that
Ping? If the current situation continues we might as well drop our nightlies since theyāre broken and unusable anyway. There needs to be a dialogue between developers and packaging. Code owners may not not be the ideal or even correct solution, but I donāt hear any other suggestions.
@ekohl the poll is conclusive, and no objections have been raised. Letās move forwards. Iāve added write access to theforeman/foreman to the packaging team on GitHub - is there anything else you need?
To clarify the result of this discussion, does this inherently give maintainer access to theforeman/foreman repository for all current and future packaging team members? I ask for two reasons:
This provides a loop-hole around the agreed upon process for becoming a maintainer and having access to the commit bit and potentially provides access to users who may not have had this before
Any future additions to the packaging team may have to come with further scrutiny since a by product would be granting core commit bit
I raise this for both understanding, vigilance and to ensure that everyone understands the full scope of this change.
Thanks for the warning. In my opinion core should trust the packaging team to do the right thing on the release branches.
If someone in packaging does not have commit access already because theyāve been an usual committer to core, I doubt they would merge anything in core, especially when not related to packaging issues (e.g - Gemfile changes, assets pipeline changes).
Generally I agree about point 2, but in any case my opinion is that much is to be gained by trusting āpackagersā to do TheRightThing*tm in core (mostly restricted to what I said in the previous paragraph) and a lot of time is to be lost by putting more barriers to that.
I agree with @dLobatog, and Iād also expect the reverse - that existing maintainers would notice if a packager goes too far, and take action. That could be a simple discussion about why a commit happened, all the way up to reverting the commit if needed. Git is a version control system, after all, we have all the history if we need it
To be honest, I think weāre taking this the wrong way. I think, commit access to foreman core should be justified by helping to maintain the codebase. I donāt want to have a loop-hole. I can only speak for myself, but the way I see it is the following: I have commit access to core, but donāt have commit access to packaging. And I think, thatās totally fine. I havenāt earned that right in foreman-packaging. This is not an issue of not trusting people (I do) or not needing help (we do), but contributors should get a proper review and people need to know the codebase and product well to make a proper review. I think commit permissions should be earned.
When releasing a plugin, one of my release todos is to send PRs to foreman-packaging to update the packages. Canāt we agree, that we only merge core PRs that have a corresponding packaging PR when needed? Ideally, a contributor would file the PR with the core PR. If thatās too much to ask, a maintainer can either help and file the PR or as someone from the packaging team for help. We donāt need any special permissions or loop-holes permissions for that.
Just my 2 cents. I hope, we find an easy solution for our issue. That is broken nightlies.
Thanks for bringing this up. Itās time for a short retrospective.
I had hoped that it would loop us into conversations that changed packaging files. It appears thereās no such granularity and you either follow the entire repository or none of it (modulo those PRās youāre participating in).
There are also quite a few PRās being merged that donāt have corresponding packaging changes, or only partially. For example https://github.com/theforeman/foreman/pull/5096 did have the dependencies packaged but there wasnāt a PR to change the package itself. If time had been invested there https://github.com/theforeman/foreman/pull/5275 was probably caught during the review phase.
Other examples can be found in the node packaging.
Overall Iām not happy with the result of the experiment because it didnāt achieve the goal I had hoped for: more cooperation between core devs and packaging.
Iād be interested in what others think, but Iām leaning to reverting the change and try to find a real solution.
My gut feeling is the that the question we need to answer is āHow does a core developer know if something needs packaging changes?ā. I believe we all want to do this better, so this isnāt a question of getting better communication, but rather how to know if such changes impact packaging.
Asking all core devs to understanding the packaging probably isnāt realistic - likewise, asking a packaging member to review every core PR isnāt either. Does this come down to more automated tests? Should we be building scratch packages for core PRs? (I know that will take a lot more Jenkins time, but Iām still trying to get us more slaves )
, so (3) it is!
Iāve never actually used this, but it looks like the kind of thing we are after.
)