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

feat: update rubocop and implement suggestions #436

Merged
merged 2 commits into from
Sep 6, 2024
Merged

Conversation

rwaffen
Copy link
Member

@rwaffen rwaffen commented Sep 6, 2024

Hi @oneiros ,

is this okay so to do? Rubocop says i should do so.
The change in the layer.rb.

cheers, Robert

@rwaffen rwaffen requested review from oneiros and a team September 6, 2024 08:11
@rwaffen
Copy link
Member Author

rwaffen commented Sep 6, 2024

okay, unit tests start failing, so i'm not supposed to do it like this 🤔

oneiros
oneiros previously approved these changes Sep 6, 2024
Copy link
Collaborator

@oneiros oneiros left a comment

Choose a reason for hiding this comment

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

That is totally fine.

@oneiros
Copy link
Collaborator

oneiros commented Sep 6, 2024

okay, unit tests start failing, so i'm not supposed to do it like this 🤔

Ouch sorry, should have taken a closer look. I made the same mistake as rubocop 🤦‍♂️

The method #present? in this case is not the one from ActiveSupport, but a custom one. That is probably why this cannot work. I fear you will have to disable the cop here.

@rwaffen
Copy link
Member Author

rwaffen commented Sep 6, 2024

okay, will add a comment to skip the cop for this line

@rwaffen rwaffen merged commit eec3554 into main Sep 6, 2024
14 checks passed
@rwaffen rwaffen deleted the update_rubocop branch September 6, 2024 09:09
@rwaffen rwaffen added the enhancement New feature or request label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants