move rubygem-jquery-ui-rails package to katello #11004
Conversation
There was a problem hiding this comment.
Technically I'd prefer to use git mv on the package directory if we're moving it. Then it also belongs in the katello comps, not foreman-plugins.
But I said if. I'd be OK with keeping this in Foreman's repo for now. Moving it creates some churn and it's really just a build-time dependency that we want to get rid of anyway. Obviously, this wouldn't be the first time we say that and then take forever to execute on it. Still, I think it's acceptable.
I think what @m-bucher meant is that currently the dependency is listed in foreman.spec but should be in rubygem-katello.spec.
MariaAga
force-pushed
the
move-jquery-ui-katello
branch
from
70527f9
to
caf2455
Compare
|
Thanks @ekohl, is this the correct way to do it? |
There was a problem hiding this comment.
I always use https://github.com/ekohl/theforeman-plumbing/blob/master/fm-packaging-update-deps to update the dependencies. Once we drop EL8 in Foreman 3.13 then we can use #8414 to automatically generate those sections based on the sources.
I wouldn't mind it if we merged the PRs and I can submit the update PRs for you. Ideally this whole complex area is going away and there's little point in learning it now.
| @@ -198,7 +198,6 @@ BuildRequires: (npm(react-intl) >= 2.8.0 with npm(react-intl) < 3.0.0) | |||
| # end package.json dependencies BuildRequires | |||
|
|
|||
| # start specfile assets BuildRequires | |||
| BuildRequires: (rubygem(jquery-ui-rails) >= 6.0 with rubygem(jquery-ui-rails) < 7.0) | |||
There was a problem hiding this comment.
I still see a line here:
https://github.com/theforeman/foreman/blob/e78bd159584f2f8d09b3d57ab94615a2ae26ecb7/bundler.d/assets.rb#L2
So our automation would place this back
There was a problem hiding this comment.
I made theforeman/foreman#10142 to remove it
| # start specfile assets BuildRequires | ||
| BuildRequires: (rubygem(jquery-ui-rails) >= 6.0 with rubygem(jquery-ui-rails) < 7.0) | ||
| # end specfile assets BuildRequires |
There was a problem hiding this comment.
We have a bit of autoupdate magic. assets in foreman.spec mapped to https://github.com/theforeman/foreman/blob/develop/bundler.d/assets.rb. We don't have bundler.d in katell. Instead, we have the section start specfile generated dependencies which maps to https://github.com/Katello/katello/blob/e99f9e93a3578886e13f9bb451e14c3d637c6880/katello.gemspec#L29-L68
There was a problem hiding this comment.
Not sure if I should keep both BuildRequires and Requires then?
There was a problem hiding this comment.
For rubygem-katello.spec (and all rubygem-*.spec really) it should autogenerate Requires, so no need for that.
MariaAga
force-pushed
the
move-jquery-ui-katello
branch
from
caf2455
to
15accfa
Compare
pr-processor
bot
added
Needs re-review
and removed
Waiting on contributor
Needs re-review
labels
|
@ekohl Thanks for the review, how can I progress this? |
|
[test rpm-copr] |
MariaAga
force-pushed
the
move-jquery-ui-katello
branch
from
15accfa
to
23a1804
Compare
MariaAga
force-pushed
the
move-jquery-ui-katello
branch
from
23a1804
to
14abf5f
Compare
for Katello/katello#10982 and theforeman/foreman#10142
I ran:
./remove_package rubygem-jquery-ui-rails./add_gem_package.sh jquery-ui-rails foreman_plugin katello