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

Adding .stickler.yml configuration file #3302

Closed
wants to merge 4 commits into from

Conversation

stickler-ci[bot]
Copy link

@stickler-ci stickler-ci bot commented Jan 17, 2019

I'm looking into the option of replacing CodeClimate with Stickler. While I know we all will miss the lovely "Your method has 7 lines (exceeds 6 allowed)", the main benefit of Stickler, that it makes fixes on its own! So no more making a commit just to fix this missing whitespace at the end of the file... 👌

Discussion: https://discuss.redash.io/t/replace-codeclimate-with-sticklerci/3037.

@ghost ghost added the in progress label Jan 17, 2019
@arikfr
Copy link
Member

arikfr commented Jan 17, 2019

I'm looking into the option of replacing CodeClimate with Stickler. While I know we all will miss the lovely "Your method has 7 lines (exceeds 6 allowed)", the main benefit of Stickler, that it makes fixes on its own! So no more making a commit just to fix this missing whitespace at the end of the file... 👌

Anyway, let's give it a shot and see if it's better or not.

@ghost ghost assigned arikfr Jan 17, 2019
@ghost ghost added the review label Jan 17, 2019
@arikfr
Copy link
Member

arikfr commented Jan 17, 2019

@jezdez
Copy link
Member

jezdez commented Jan 17, 2019

Neat! +1

@@ -60,7 +60,7 @@ def create_redis_connection():

return client


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool 😁

@arikfr
Copy link
Member

arikfr commented Jan 22, 2019

After some additional thought I realized that while the autofix feature is really nice, it has the potential to create confusion for contributors:

  1. They push some code.
  2. Stickler fixes lint issues.
  3. They try to push again and get an error (because remote changed).

At this point, if they aren't familiar with Git enough, they might get confused/frustrated. The value we get from the auto fixes doesn't seem to worth this friction point.

👋 Stickler.

I will still put some effort in either replacing CodeClimate with a less noisy tool or adjust its settings.

@arikfr arikfr closed this Jan 22, 2019
@ghost ghost removed in progress labels Jan 22, 2019
@arikfr arikfr deleted the add-stickler-config branch January 22, 2019 07:49
@jezdez
Copy link
Member

jezdez commented Jan 22, 2019

@arikfr It would be neat if Stickler would just use the new change suggestions instead of fixing the problems outright.

@arikfr
Copy link
Member

arikfr commented Jan 22, 2019

@jezdez actually this is what I was looking for initially. But couldn't find such a solution. It's possible that there is no API for this feature yet, because it's still in beta.

@jezdez
Copy link
Member

jezdez commented Jan 22, 2019

@arikfr Ah I forgot it's in beta, true. I'd be surprised if Stickler wouldn't use it once it's in GA.

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.

4 participants