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

Fix rubocop warnings #2991

Merged
merged 10 commits into from
Dec 6, 2023
Merged

Conversation

robertcheramy
Copy link
Collaborator

Pre-Request Checklist

  • Passes rubocop code analysis (try rubocop --auto-correct)

Description

This PR fixes a lot of rubocup warnings.
I did not fix all warnings in .rubocop_todo.yml, as I did not felt safe doing so. It would be a better Idea to fix them when touching this part of the code.
I preferred small commits in order to find out a problem easier if there is one. It tried to test as much as possible.

Closes issue #2983

Rescuefail is a constant and rubocop says:
Naming/ConstantName: Use SCREAMING_SNAKE_CASE for constants.
Mostly autocorrection and what I felt safe to change. There are still
warnings beeing suppressed in .rubocop_todo.yml, but this should be in a
separate commit.
- set_cmd isn't realy a pure writer method. Renaming it to process_cmd
- mocha does not work with the latest minitest version (rake test won't
run), bumping it to Version 2.1
- Leaving extra/rest_client.rb unchanged, as I can't test the syslog
functionality for now
- Oxidized::Config::* are all constants
- As there were different namings for the rescue exception variables in
the code, make them consistent and use "e".
- gitcrypt.rb has a nested rescue whih keeps a different variable name
(create_error)
- No need to specify Oxidized:: as we already are in the Oxidized Module
- Not touching lib/oxidized/config/vars.rb as I don't feel sure about it
The class Nodes inherits Array and as such has the method empty?
implemented
@robertcheramy
Copy link
Collaborator Author

@ytti and @aschaber1 - if would be great if you would find time to look into this PR.
@aschaber1 - I have changed a member name in refinements.rb (Commit f60ec93)
@ytti - I changed constant names in different commits, commented suppressed exceptions, and so on - that's a lot, and hope this is OK for you.

@robertcheramy robertcheramy self-assigned this Nov 27, 2023
@robertcheramy robertcheramy linked an issue Nov 27, 2023 that may be closed by this pull request
@robertcheramy
Copy link
Collaborator Author

I plan to merge this PR next week (starting 11.12.2023) if there is no reaction from the reviewers until then.

Copy link
Collaborator

@aschaber1 aschaber1 left a comment

Choose a reason for hiding this comment

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

Havent tested it, but looking at the PR: LGTM

@ytti
Copy link
Owner

ytti commented Dec 6, 2023

I thought I found serious issue, but I think i''m wrong. I don't see problems, but I'm also not confident as I didn't spend much time and won't spend. But at least someone is doing something, and if something breaks, as long as it gets fixed, that's still progress.

@aschaber1 aschaber1 merged commit e81eb77 into ytti:master Dec 6, 2023
5 checks passed
@robertcheramy robertcheramy deleted the 2983_fix_rubocop_warnings branch January 31, 2024 06:37
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.

Fix rubocup warnings
3 participants