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.

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

Update gem versions in modules #152

Merged
merged 8 commits into from
Nov 3, 2023
Merged

Conversation

@ekohl
Copy link
Member Author

ekohl commented Sep 28, 2023

Still needs voxpupuli/puppet-lint-spaceship_operator_without_tag-check#18 now. Will send a batch of PRs when it's released.

@ekohl
Copy link
Member Author

ekohl commented Sep 28, 2023

Somehow bundler can't find version 1.0.0 so I'm still stumped.

These are either obsolete or included in voxpupuli-puppet-lint-plugins,
which is pulled in via voxpupuli-test.
In some modules we hav spec/fixtures/hiera which would otherwise end up
ignored.
ekohl added 2 commits October 9, 2023 23:24
In practice this leads to the job being disabled and we don't check the
results, which means it doesn't really help us.
gem 'rake'

<% (@configs['required'] + (@configs['extra'] || [])).each do |gem| -%>
gem '<%= gem['gem'] %>'<%= ", '#{gem['version']}'" if gem['version'] %><%= ", #{gem['options'].inspect}" if gem['options'] %><%= " if RUBY_VERSION #{gem['ruby_version']}" if (gem['ruby_version']) %>
<% end -%>

# Pin rdoc to prevent updating bundled psych (https://github.com/ruby/rdoc/commit/ebe185c8775b2afe844eb3da6fa78adaa79e29a4)
gem 'rdoc', '< 6.4'
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we keep this on Ruby 2 in to prevent updating psych on Ruby 2.7?

Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to merge it without it and then see

@ekohl ekohl marked this pull request as ready for review October 12, 2023 16:07
@ekohl
Copy link
Member Author

ekohl commented Oct 12, 2023

I think this is ready for review now. All PRs have been updated and I expect them to pass now.

@ekohl
Copy link
Member Author

ekohl commented Oct 13, 2023

Still required to fix:

Comment on lines -6 to -7
schedule:
- cron: '4 4 * * *'
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is so that "no changes to main branch for 60 days" doesn't disable the workflow?
Should we have a second workflow that is doing just schedule?
(non-blocking)

Copy link
Member Author

Choose a reason for hiding this comment

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

I also debated that. In practice I rarely look at it and @ehelms wasn't even aware that we had it. So for now I'm leaning to keeping it disabled.

Copy link
Member

Choose a reason for hiding this comment

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

k, wfm

(I did use it the other day when for some reason the centos:stream8 container stopped booting out of a sudden to double check that it really worked a few hours ago, but then again this was probably the only use of it by me :D)

Copy link
Member Author

Choose a reason for hiding this comment

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

I also used the other day to see that puppet-katello was already failing before I got started, but I wonder if it still makes sense. At least with the reusable workflows we'll have less duplication if we have 2 separate files.

Copy link
Member

Choose a reason for hiding this comment

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

technically, you could even re-use a re-usable workflow: ci.yml.erb becomes workflow.yml.erb with on: workflow_call and then we have ci.yml and cron.yml that both call that…

or you use ERB to generate two "identical" files with different on statements.

or I shut up and we get it in as is and think later how to overengineer this ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

You may be able to trick modulesync with using a symlink and read __FILE__ in the on condition. Does that qualify as overengineering? ;)

Copy link
Member

Choose a reason for hiding this comment

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

It's a "smart" solution. "smart" solutions tend to explode in spectacular ways down the road. ;-)

gem 'rake'

<% (@configs['required'] + (@configs['extra'] || [])).each do |gem| -%>
gem '<%= gem['gem'] %>'<%= ", '#{gem['version']}'" if gem['version'] %><%= ", #{gem['options'].inspect}" if gem['options'] %><%= " if RUBY_VERSION #{gem['ruby_version']}" if (gem['ruby_version']) %>
<% end -%>

# Pin rdoc to prevent updating bundled psych (https://github.com/ruby/rdoc/commit/ebe185c8775b2afe844eb3da6fa78adaa79e29a4)
gem 'rdoc', '< 6.4'
Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea.

@ekohl ekohl merged commit e646f4e into theforeman:master Nov 3, 2023
2 checks passed
@ekohl ekohl deleted the update-gems branch November 3, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants