Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add packit support #934

Merged
merged 1 commit into from
May 7, 2024
Merged

Add packit support #934

merged 1 commit into from
May 7, 2024

Conversation

ianballou
Copy link
Member

I used hammer-cli-foreman's packit file as an example.

@ianballou
Copy link
Member Author

/packit build

@ianballou
Copy link
Member Author

/packit test

chris1984
chris1984 approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@ianballou
Copy link
Member Author 

+ scl enable '%{scl}' -
/var/tmp/rpm-tmp.Y6Kh7H: line 31: scl: command not found
error: Bad exit status from /var/tmp/rpm-tmp.Y6Kh7H (%prep)
    Bad exit status from /var/tmp/rpm-tmp.Y6Kh7H (%prep)

Why are SCLs trying to be enabled here...

@ianballou
Copy link
Member Author

theforeman/foreman-packaging#10731 will fix the builds

ekohl
ekohl suggested changes May 2, 2024
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also drop the test deps from the gemspec to avoid duplication

gemfile.d/test.rb Outdated
@@ -0,0 +1,8 @@
group :test do
gem 'ci_reporter', '>= 1.6.3', "< 2.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was for Jenkins. I don't think it supports GHA

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're dropping the use of it, you can also drop the dependency

Suggested change
gem 'ci_reporter', '>= 1.6.3', "< 2.0.0"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized you probably forgot to delete this file

Gemfile Outdated
@@ -7,3 +7,7 @@ gemspec
local_gemfile = File.join(File.dirname(__FILE__), file_name)
self.instance_eval(Bundler.read_file(local_gemfile)) if File.exist?(local_gemfile)
end

Dir[File.join(__dir__, 'gemfile.d', '*.rb')].each do |bundle|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for this here. You can just define it in the Gemfile itself. Gemfile.d is for Foreman plugins.

While we don't use it today, dependabot doesn't support eval_gemfile so I'd stick to a plain Gemfile.

Though the instance_eval above already breaks dependabot.

@ianballou ianballou requested review from chris1984 and ekohl May 6, 2024 16:54
@ianballou
Copy link
Member Author

This PR should fix our nightly builds, which folks are waiting to test.

@ianballou ianballou force-pushed the add-packit-support branch 2 times, most recently from 78c2617 to 040a1ca Compare May 6, 2024 17:01
@ianballou
Copy link
Member Author

@ekohl thanks for catching the gemfile folder, it's deleted now.

@ianballou
Copy link
Member Author

Did a quick test, my PR doesn't seem to break anything in h-c-k at least.

@ianballou
Copy link
Member Author

Merging since it should help with nightly, but we can make further improvements as needed.

@ianballou ianballou merged commit a2a12d3 into Katello:master May 7, 2024
7 of 10 checks passed
@ianballou ianballou deleted the add-packit-support branch May 7, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris1984 chris1984 chris1984 approved these changes

@ekohl ekohl Awaiting requested review from ekohl

Assignees
No one assigned
Labels
Waiting on contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ianballou @ekohl @chris1984