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

Lots of RuboCop offenses #33

Closed
zacholauson opened this issue Dec 12, 2015 · 8 comments
Closed

Lots of RuboCop offenses #33

zacholauson opened this issue Dec 12, 2015 · 8 comments

Comments

@zacholauson
Copy link

I would be happy to help either disable some of the active RuboCop inspections, or change the code to follow the default inspections.

Do you guys have any preference on how to solve all of these?

@solnic
Copy link
Member

solnic commented Dec 12, 2015

Hey, yes I'd accept a PR that fixes the issues with the exception of .() calls, I love it and I want to make it more popular, esp. that you can attach some extra meaning to it, ie. such calls could mean a side-effect-less call that is functional. I hope @AMHOL can get used to it? 😂

@zacholauson
Copy link
Author

Awesome. I like the .() call syntax as well. I haven't seen it used very much so its cool to see it used so consistently in this project.

I'll start working through the offenses and post back if I have any questions about desired code style.

Thanks!

@zacholauson
Copy link
Author

@solnic / @AMHOL, I opened a work in progress pull request as I work through the offenses. Initially I disabled the inspection that prefers .call() over .(), but we could also change this inspection to enforce .() over .call(). What would you guys prefer?

@solnic
Copy link
Member

solnic commented Dec 12, 2015

I'd say we should simply disable this check
On Sat, 12 Dec 2015 at 20:05, Zach Olauson notifications@github.com wrote:

@solnic https://github.com/solnic, I opened a work in progress pull
request as I work through the offenses. Initially I disabled the inspection
that prefers .call() over .(), but we could also change this inspection
to enforce .() over .call(). What would you prefer?


Reply to this email directly or view it on GitHub
#33 (comment)
.

@zacholauson
Copy link
Author

Sounds good. Thanks @solnic

@AMHOL
Copy link
Member

AMHOL commented Dec 13, 2015

@solnic @zacholauson I can live with it lol

@solnic solnic modified the milestone: v0.4.0 Dec 15, 2015
@zacholauson
Copy link
Author

Hey guys, just as an update. I've been home visiting family so I haven't been working on this much. I plan to continue to work on it this weekend, I have about 10 offenses left.

@solnic
Copy link
Member

solnic commented Jan 11, 2016

No worries man :) BTW a lot of code has been moved to dry-logic.

@solnic solnic removed this from the v0.4.0 milestone Jan 11, 2016
@solnic solnic closed this as completed in 7026366 Mar 7, 2016
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

No branches or pull requests

3 participants