Skip to content

Fix Rubocop offenses#92

Merged
stas merged 2 commits intostas:masterfrom
mamhoff:correct-rubocop-performance-offenses
Oct 8, 2023
Merged

Fix Rubocop offenses#92
stas merged 2 commits intostas:masterfrom
mamhoff:correct-rubocop-performance-offenses

Conversation

@mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Jul 21, 2023

What is the current behavior?

Currently, builds are failing because of Rubocop complaining about three lines.

What is the new behavior?

The build still fails because of Ransack 4 being incompatible with the library, but that's for a different PR.

Checklist

Please make sure the following requirements are complete:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes /
    features)
  • All automated checks pass (CI/CD)

Currently, builds are failing because of Rubocop complaining about these
three lines.
@mamhoff mamhoff changed the title Rubocop fix: Use filter_map instead of map { ... }.compact Fix specs Jul 21, 2023
@mamhoff mamhoff force-pushed the correct-rubocop-performance-offenses branch 2 times, most recently from a109bd9 to 0e7fd51 Compare July 21, 2023 08:44
@mamhoff mamhoff changed the title Fix specs Fix Rubocop offenses Jul 21, 2023
@stas
Copy link
Owner

stas commented Jul 21, 2023

Ty ty @mamhoff

Happy to merge this one and leave the tests fixing for another task. Lmk wdyt?

@mamhoff
Copy link
Contributor Author

mamhoff commented Jul 21, 2023

Sure, I tried fixing the specs with Ransack 4, but that's a whole different beast.

Array#filter_map is only introduced in Ruby 2.7. This ensures we're
still supporting the 2.x series, but drops support for the now pretty
old Ruby 2.6.
@mamhoff
Copy link
Contributor Author

mamhoff commented Jul 24, 2023

So... filter_map is only supported from Ruby 2.7. I've added a commit to run the specs with Ruby 2.7 instead of 2.6. This means this PR will effectively drop Ruby 2.6 support.

The other option would be to ignore the cop. Your call :)

@stas stas merged commit 886218e into stas:master Oct 8, 2023
@stas
Copy link
Owner

stas commented Oct 8, 2023

Thank you @mamhoff

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