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 style violations #1021

Closed
nodo opened this Issue Sep 15, 2016 · 24 comments

Comments

Projects
None yet
7 participants
@nodo
Copy link
Member

nodo commented Sep 15, 2016

At the moment, the codebase has several style violations. It would be great to refactor the code, such that we are up to date with the community best practices https://github.com/bbatsov/ruby-style-guide.

The list of the violations is in two files:

These files specify the ignored violations. It would be great to remove some of the violations from the files and fix them progressively in order to keep the PRs small.

pmatsinopoulos pushed a commit to pmatsinopoulos/cucumber-ruby that referenced this issue Sep 20, 2016

pmatsinopoulos added a commit to pmatsinopoulos/cucumber-ruby that referenced this issue Sep 23, 2016

nodo added a commit that referenced this issue Sep 28, 2016

Merge pull request #1027 from pmatsinopoulos/fix-style-violation-1021…
…-style-string-literals

Refs #1021 - Fixes the rubocop violation 'Style/StringLiterals'

@phoebeclarke phoebeclarke referenced this issue Oct 1, 2016

Merged

Fixes for rubocop violations #1042

1 of 7 tasks complete

@pmatsinopoulos pmatsinopoulos referenced this issue Oct 5, 2016

Closed

Fixes Style/IndentArray #1045

2 of 8 tasks complete

@phoebeclarke phoebeclarke referenced this issue Oct 7, 2016

Closed

Fixes for rubocop violations #1048

1 of 7 tasks complete

@nodo nodo added the newcomers label Jan 27, 2017

@bv bv referenced this issue Mar 12, 2017

Merged

Fix some rubocop offenses 1021 001 #1093

1 of 3 tasks complete

@bv bv referenced this issue Mar 22, 2017

Merged

Fix rubocop offences 1021 002 #1095

1 of 3 tasks complete

@bv bv referenced this issue Apr 12, 2017

Merged

Fix rubocop offenses 1021 003 #1107

1 of 3 tasks complete

@mattwynne mattwynne removed the Easy label Jun 30, 2017

@stale

This comment has been minimized.

Copy link

stale bot commented Mar 25, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the Stale label Mar 25, 2018

@xtrasimplicity

This comment has been minimized.

Copy link
Member

xtrasimplicity commented Mar 25, 2018

Reopened. :)

@stale stale bot removed the Stale label Mar 25, 2018

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Mar 25, 2018

Thanks, @xtrasimplicity !

@stale

This comment has been minimized.

Copy link

stale bot commented May 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the Stale label May 24, 2018

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented May 24, 2018

I will get back to this eventually!

@stale stale bot removed the Stale label May 24, 2018

@stale

This comment has been minimized.

Copy link

stale bot commented Jul 23, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the Stale label Jul 23, 2018

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 23, 2018

Aaaand keeping open again! (Sorry all!)

@stale stale bot removed the Stale label Jul 23, 2018

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 24, 2018

32 commits with more fixes have been made. (Many more to come)

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 25, 2018

Another 23 commits/cops down!

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 25, 2018

I should add that all the auto-correctable cops are now done. The next ones will require some more work and others I'll need to make some style decisions around things like line lengths and complexities. I may just leave the complexity counters where they're at as the repo has some pretty monstrous module/classes in it. Part of that size may simply be necessitated by the work we're doing, and that's a-okay. :)

23 cops left!

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 27, 2018

Alrighty! Down to the final 8 cops left in the todo file! These are going to be tough to address as they'll require some serious refactoring to handle.

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 27, 2018

With a85672c, I'm calling this issue FINALLY complete! I'll leave it open for a day for feedback/changes.

For some of the limit based cops, I chose a number that would handle the bulk of cases and then either inline disabled some or refactored. There were some scenarios where I wasn't sure how to address them and instead inline disabled. One of the main ones to point out would be setting up a responds_to_method_missing? method here. If anyone wants to take a crack at it or wants to let me know the best way to handle it, feel free. :)

Thanks!

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Jul 30, 2018

Turns out that the code changes made aren't suitable for use with older Ruby versions so I need to spend some time fixing that. It's mainly just updating the uses of .match? to use something supported by those versions.

@jaysonesmith

This comment has been minimized.

Copy link
Member

jaysonesmith commented Aug 5, 2018

With @brasmusson 's changes to the match usage, this can now be considered complete! Rubocop has been a part of the rake task for a bit now, so any new development should conform to the rules. If anyone would like to go over the current rubocop file's settings, feel free to reach out or tag me in an issue!

@nodo

This comment has been minimized.

Copy link
Member

nodo commented Aug 5, 2018

🎉

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