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 some cops #11064

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

archanaserver
Copy link
Contributor

What are the changes introduced in this pull request?

Based on #11009

This pull request addresses the Style/TrailingCommaInArrayLiteral RuboCop offense by ensuring that all multiline array literals in the codebase have a trailing comma after the last item.

Considerations taken when implementing this change?

Ensured that all multiline array literals are updated uniformly with trailing commas.
Verified that the changes do not impact existing functionality or cause syntax errors.

What are the testing steps for this pull request?

bundle exec rubocop

@ianballou
Copy link
Member

This PR has no changes in it.

@archanaserver archanaserver force-pushed the fix-style/trailingcommainarrayliteral branch from 4dd1e7e to fe30aad Compare July 22, 2024 12:48
@archanaserver archanaserver changed the title Fix Style/TrailingCommaInArrayLiteral cop Fix some cops Jul 22, 2024
@ianballou
Copy link
Member

I think a rebase is needed.

@archanaserver archanaserver force-pushed the fix-style/trailingcommainarrayliteral branch 3 times, most recently from 7efa86e to ecaf9c9 Compare July 23, 2024 07:50
@archanaserver
Copy link
Contributor Author

@ianballou so i tested with this changes 7efa86e, and it is working fine, at least rubocop is green

@archanaserver archanaserver force-pushed the fix-style/trailingcommainarrayliteral branch from ecaf9c9 to 2d32117 Compare July 25, 2024 18:52
@archanaserver
Copy link
Contributor Author

@ianballou so i tested with this changes 7efa86e, and it is working fine, at least rubocop is green

again it is passing rubocop, with 7efa86e changes. we can merge this one @ianballou.

@ianballou
Copy link
Member

Seeing one more rubocop issue:

Error: unrecognized cop Lint/ConstantDefinitionInBlock found in .rubocop_todo.yml, unrecognized cop Lint/RedundantSafeNavigation found in .rubocop_todo.yml, unrecognized cop Lint/UselessMethodDefinition found in .rubocop_todo.yml, unrecognized cop Style/CombinableLoops found in .rubocop_todo.yml

@archanaserver
Copy link
Contributor Author

Seeing one more rubocop issue:

Error: unrecognized cop Lint/ConstantDefinitionInBlock found in .rubocop_todo.yml, unrecognized cop Lint/RedundantSafeNavigation found in .rubocop_todo.yml, unrecognized cop Lint/UselessMethodDefinition found in .rubocop_todo.yml, unrecognized cop Style/CombinableLoops found in .rubocop_todo.yml

@ianballou it is due to these cops are not compatible with the rubocop version we have in this branch, but i tested with this changes f6a162b and it's all green, may be we can merge this one and see it in #11009. i'm not sure if we can do this, like merging this with rubocop failure? WDYT?

@archanaserver archanaserver force-pushed the fix-style/trailingcommainarrayliteral branch from b955f88 to f3f34f6 Compare July 31, 2024 09:48
@archanaserver
Copy link
Contributor Author

@ianballou this is what i was talking about, if we combine #11064 and #11009, passing all rubocop tests #11093

@archanaserver archanaserver force-pushed the fix-style/trailingcommainarrayliteral branch from f3f34f6 to 5781756 Compare August 9, 2024 07:31
@archanaserver
Copy link
Contributor Author

@ianballou this is what i was talking about, if we combine #11064 and #11009, passing all rubocop tests #11093

@ianballou in case you missed it, any thoughts on the recent changes?

@ianballou
Copy link
Member

@archanaserver rubocop is still failing, was that intentional?

@archanaserver
Copy link
Contributor Author

@archanaserver rubocop is still failing, was that intentional?

so the changes i have made here, few cops need gem 'theforeman-rubocop', '~> 0.1.0', so that's why the failure was occuring, but i'm trying to do this on our main PR now #11009, where we already upgrade the gem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants