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

Introduce RuboCop #267

Merged
merged 1 commit into from
Jun 23, 2023
Merged

Introduce RuboCop #267

merged 1 commit into from
Jun 23, 2023

Conversation

bastelfreak
Copy link
Member

@bastelfreak bastelfreak commented Mar 17, 2023 •
edited

Contains #268

@bastelfreak bastelfreak self-assigned this Mar 17, 2023
@bastelfreak bastelfreak requested a review from ekohl March 17, 2023 08:43
ekohl
ekohl requested changes Mar 17, 2023
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.

You appear to have squashed various changes into a single commit.

@bastelfreak bastelfreak changed the title Drop Ruby 2.5/2.6 support Drop Ruby 2.5/2.6 support and update RuboCop Mar 17, 2023
@bastelfreak
Copy link
Member Author

@ekohl I moved the dependabot config to another PR and updated the PR title. Is that good now?

@bastelfreak bastelfreak force-pushed the ruby27 branch 2 times, most recently from 13ae824 to 6b92985 Compare March 17, 2023 10:19
@bastelfreak bastelfreak mentioned this pull request Jun 21, 2023
Merged
@bastelfreak bastelfreak changed the title Drop Ruby 2.5/2.6 support and update RuboCop Introduce RuboCop Jun 21, 2023
@bastelfreak
Copy link
Member Author

I split the commits up and pulled "dropping ruby 2.5/26." into #274

ekohl
ekohl reviewed Jun 21, 2023
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 wonder if we should disable the cop that uses single quotes to keep the diff smaller. These days there is no performance difference so it really doesn't matter. But I'm fine either way.

Gemfile Outdated Show resolved Hide resolved
Rakefile
@@ -49,50 +48,50 @@ def factset_to_os_label(fs)
os_rel = fs.fetch(:operatingsystemmajrelease, fs.fetch(:lsbmajdistrelease, nil))
os__rel = fs.fetch(:lsbdistrelease, fs.fetch(:operatingsystemrelease, '@@@'))
else
require 'pp'
Copy link
Member

Choose a reason for hiding this comment

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

Is this loaded by default?

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 think that's indeed in ruby core and the require isn't required anymore. In my tests it works fine.

facts/Gemfile Outdated Show resolved Hide resolved
rakelib/rhel_alts.rake Outdated Show resolved Hide resolved
@bastelfreak bastelfreak merged commit 91d03da into voxpupuli:master Jun 23, 2023
6 checks passed
@bastelfreak bastelfreak deleted the ruby27 branch June 23, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl requested changes

@jay7x jay7x jay7x approved these changes

Assignees

@bastelfreak bastelfreak

Labels
enhancement
Projects
Implement Ruby 3.2 support
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bastelfreak @ekohl @jay7x