Skip to content

Conversation

@chelnak
Copy link

@chelnak chelnak commented Oct 4, 2022

The aim of this PR is to implement the suggestions provided by rubocop now that the TargetVersion has been bought inline with the minimum required ruby version.

This PR only focuses on code in lib and aims to ensure that the spec tests remain green.

There will be a followup PR that contains fixes for code in spec.

Additionally the following changes have been added to the PR as optional extras.

Refactor load_from_gems

Previously load_from_gems contained nested loops and block continuation. It was hard to read and probably not that efficient when processing many files.

This PR adds a commit that cleans the method up by splitting the operations out in to three distinct operations, hopefully simplifying the code slightly.

Refactor line length check

Prior to this PR there was some code duplication between the 140char and 80char line length checks.

This PR contains a commit that breaks out the core logic for these checks in to its own class.
This reduces duplication and also allows for a configurable line length.

In the future this could be expanded to allow the user to specify their desired line length from config or a flag.

@chelnak chelnak requested a review from a team as a code owner October 4, 2022 13:35
@chelnak chelnak force-pushed the maint-rubocop_updates branch 6 times, most recently from ac64181 to a1b0d4a Compare October 4, 2022 19:02
@pmcmaw
Copy link

pmcmaw commented Oct 4, 2022

It looks like only the CLA is running here and the other checks aren't

@chelnak
Copy link
Author

chelnak commented Oct 4, 2022

#53 fixes that 🙂

chelnak added 15 commits October 5, 2022 13:33
Previously load_from_gems contained nested loops and block
continuation. It was hard to read and probably not that efficient when
processing many files.

This commit cleans the method up by splitting the operations out in to
three distinct operations, hopefully simplifying the code slightly.

In addition, the Rubocop fixes have also been applied in this commit.
Prior to this commit there was some code duplication between the
140char and 80char checks.

This PR breaks out the core logic for these checks in to it's own class.
This reduces duplication and also allows for a configurable line length.

In the future this could be expanded to allow the user to specify their
desired line length from config or a flag.
@chelnak chelnak force-pushed the maint-rubocop_updates branch from a1b0d4a to dfa3b47 Compare October 5, 2022 12:33
@GSPatton
Copy link

GSPatton commented Oct 5, 2022

since this is a pretty big PR, could you add a description detailing what the PR achieves? And add a label?

@chelnak
Copy link
Author

chelnak commented Oct 6, 2022

My bad @GSPatton

@chelnak chelnak self-assigned this Oct 6, 2022
@david22swan david22swan merged commit b028af7 into main Oct 6, 2022
@david22swan david22swan deleted the maint-rubocop_updates branch October 6, 2022 10:12
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.

5 participants