Fixes #37595 - Drop defunct deb mirror_publication_options #11044
Conversation
|
Looks good to me. |
There was a problem hiding this comment.
Cleans up some dead code. From using git grep, it is clear only deb content has
mirror_publication_optionsdefined, but ever since https://projects.theforeman.org/issues/35959 deb content no longer uses those publication options, because a verbatim publication is used instead.
Hey @quba42 ! It's possible I'm missing some context, but from https://projects.theforeman.org/issues/35959 I found the unmerged draft PR #10420 -- should that go in first?
|
@wbclark This change makes sense independently of the changes in #10420 and is ready now. Once this is merged, I will soon rebase an updated version of #10420 on top of this change. I found this change while working on #10420, but decided to open this PR separately, because it is small, easy to review in isolation and unrelated to the structured APT feature. |
Ever since we switched to verbatim publications for deb content on smart proxies this code path has been unused. We could have cleaned it up as part of the original change in 4a363fd, so I am doing so now.
quba42
force-pushed
the
cleanup_apt_mirror_publication_options
branch
from
220e756
to
116ef42
Compare
There was a problem hiding this comment.
Thanks for the additional context.
The code looks fine and worked for me in some basic testing. I can see that the apt mirror_publication_options is not reachable via any code path. I didn't see anything in test/services/katello/pulp3/* that would require any update to accommodate this change, and the tests are green.
I have one (non-blocking) observation/idea (see comment below), and I'm honestly fine with merging this either way so I'll go ahead and ACK it and you can make the final call. ;-)
Ever since we switched to verbatim publications for deb content on smart proxies this code path has been unused. We could have cleaned it up as part of the change in 4a363fd.
What are the changes introduced in this pull request?
Cleans up some dead code. From using git grep, it is clear only deb content has
mirror_publication_optionsdefined, but ever since https://projects.theforeman.org/issues/35959 deb content no longer uses those publication options, because a verbatim publication is used instead.What are the testing steps for this pull request?
One could sync some deb content and some non deb content to smart proxy as a test, but it is pretty clear from the code that this change has no effect.