Refactor ruby files to follow Ruby Style Guide

[moki298]

This is referenced from the issue: #6314.

Updated:

Purpose:

To remove linting errors using rubocop

Current Plan

Since currently there are more than 6000 offenses I would like to break this into smaller issues, also I would like to track them all in this issue.

Active issues as of now are listed below, please feel free to create a new issue for any other directory and start refactoring the ruby files in that directory. Please reference this issue in every issue that you create for each directory so that we will have a single issue to track the progress.

Related Issues

Issue Id	Files/Directory Involved	PR Id/Progress
None	/app/api/srch	#6380 Work in progress
#6416	/app/api & /app/channels	#6425 Merged
#6417	/app/controllers	#6485 Work in progress
#6436	/app/helpers	#6442 Work in progress
#6509	/app/mailers	#6515 Merged

Rubocop help

• Download rubocop from here
• Once successfully downloaded run rubocop command inside the directory, this lists all the offenses in ruby files in this directory. More info here
• Running rubocop -a fixes most of the issues automatically

[welcome]

Thanks for opening your first issue! This space is protected by our Code of Conduct - and we're here to help.
Please follow the issue template to help us help you 👍🎉😄
If you have screenshots or a gif to share demonstrating the issue, that's really helpful! 📸
Do join our Gitter channel for some brainstorming discussions.

[moki298]

I used rubocop and found many violations, @newbazz do you think starting resolving them is a good place to start with this issue?

[newbazz]

yes i think this is a good first step, you can file an issue and start working on the same. @moki298

[jywarren]

Yes please, this would be great!

…

On Mon, Sep 30, 2019 at 5:31 AM newbazz ***@***.***> wrote: yes i think this is a good first step, you can file an issue and start working on the same. @moki298 <https://github.com/moki298> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6340?email_source=notifications&email_token=AAAF6J5JOZF7TGKIZAXTCL3QMHBLRA5CNFSM4I27DZ7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD75BCHA#issuecomment-536482076>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J2FRC3DBTGSA656KEDQMHBLRANCNFSM4I27DZ7A> .

[moki298]

Ok, I found around 6000 violations across the entire repository, I don't want to do this in a single issue/PR as it would be cumbersome for the reviewers, any thoughts on that? @jywarren @newbazz

Is it any better if we do this single directory at a time? Also, how can I test locally once I do any changes? Is there a defined workflow for this?

[camillesk]

I'm interest to help on this issue! I thin we can fix by directories, setting a maximum of files that we can change in which PR

[discombobulateme]

@moki298 woooow that's an impressive work :)
How reliable is using rubocop? Does it works like a filter than if should be manually checked? Would love to learn how to use it and cooperate :)

[moki298]

@camillesk @discombobulateme thanks for your interest. I believe we should break this into smaller issues. But I am waiting for comments from the @jywarren @newbazz to know what they think about this as I am new to this repo and ruby world. I would like to wait and see for a week or so and then I would start breaking this into smaller issues.

Meanwhile, it would be great if you have the environment setup and ready. @discombobulateme I think rubocop it the best way to update this codebase, manual checking is ineffective for dealing with 6000 plus offenses.

Clone this repo and install the required software and then install rubocop with instructions from here and then run rubocop from this repo you should be able to see all the offenses listed.

[discombobulateme]

Thanks @moki298 ! I'll do it within the next hour ;)

[newbazz]

this is a good first step towards refactoring the ruby code, I would like someone to mention run the linter rubycorp before making a pull request in the contributing guidelines as well.

[moki298]

Ok, I will go ahead and start breaking this into smaller issues, @discombobulateme @camillesk please find the appropriate issues tagged on this topic and start working on them. Please let me know if you are stuck with something. Thanks.

[bbborisk]

I would like to start work on #6417 if it is OK?

[newbazz]

you can comment on that issue, and wait for the current assignee to get back to you.

[bbborisk]

Thanks!

[davemenninger]

Should we be looking at everything in the .rubocop_todo.yml or just what .rubocop.yml reports?

rubocop -c .rubocop_todo.yml -a fixes a large proportion of issues automatically...

[rojasTob]

Hi @davemenninger, finally what file did you run for this issue? .rubocop_todo.yml or .rubocop.yml ?

[moki298]

Hi @davemenninger, finally what file did you run for this issue? .rubocop_todo.yml or .rubocop.yml ?

You should be using .rubocop.yml file to resolve the lint offenses, not rubocop_todo.yml.

[moki298]

@jywarren @newbazz can you please have a look at all the pending PR's and comment/merge them.

[oedbro]

I was going to create a new issue for this, but got a bunch of issues with the .rubocop_shopify_styleguide.yml. Can I begin by fixing those issues?

[cesswairimu]

Thanks all for the work, the planning and all the help here ❤️ ❤️. The remaining open issue from the breakdown is this one . I will close this and make ftos of the remaining files from that issue. Thanks