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

Ameba: Handle Naming/BlockParameterName #9

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Ameba: Handle Naming/BlockParameterName #9

merged 1 commit into from
Aug 26, 2024

Conversation

walro
Copy link
Contributor

@walro walro commented Aug 26, 2024

WHY are these changes introduced?

CI fails as Ameba runs on 1.6.1 and the rule was added in 1.6.0:

https://github.com/crystal-ameba/ameba/releases/tag/v1.6.0

WHAT is this pull request doing?

Taking care of Naming/BlockParameterName offenses. Decided to not change the p as parser is already taken in scope and option_parser just felt worse than the disabling (disable also affects less lines).

HOW can this pull request be tested?

Specs

@walro walro requested a review from a team as a code owner August 26, 2024 11:03
@walro
Copy link
Contributor Author

walro commented Aug 26, 2024

Ok, now new problems pop up as CI got further, but I think we should adress in separate PRs.

@walro walro requested a review from spuun August 26, 2024 11:07
Copy link
Contributor

@oskgu360 oskgu360 left a comment

Choose a reason for hiding this comment

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

Hmm, some CI builds fails 😢

But for this change I guess this is all we need to look at!
Screenshot 2024-08-26 at 13 05 40

@walro
Copy link
Contributor Author

walro commented Aug 26, 2024

Haha! Nice thinking there sir

@walro
Copy link
Contributor Author

walro commented Aug 26, 2024

I think the fedora-39 build is stuck somehow, will just merge this now.

@walro walro merged commit ec095f2 into main Aug 26, 2024
20 of 23 checks passed
@walro walro deleted the lint branch August 26, 2024 11:57
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