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

Add rubocop into Travis-CI #284

Closed

Conversation

colby-swandale
Copy link
Member

Note: This PR depends on #282 being merged first.

This PR adds a step in Travis to run rubocop. This would be very beneficial to the project because it makes sure that any new changes to rake conform to the existing style guide.

@coveralls
Copy link

coveralls commented Nov 2, 2018

Coverage Status

Coverage decreased (-0.2%) to 97.574% when pulling 606e613 on colby-swandale:colby/add-rubocop-into-ci into f0c7528 on ruby:master.

@colby-swandale
Copy link
Member Author

This has been rebased with master

@yuki24
Copy link
Member

yuki24 commented Nov 6, 2018

I'd personally prefer to add lint checks to git hooks as it catches syntactical mistakes early. Lint checks on CI create a long feedback loop depending on how many jobs are in the queue.

@colby-swandale
Copy link
Member Author

colby-swandale commented Nov 6, 2018

Thanks for the feedback @yuki24. I think both are good practices to have in a project. Having Rubocop in a git hook and having a quick feedback loop is super handy for developers. But also having a final check before any code is merged helps prevent any mistakes from getting into master.

Not everyone likes/uses git hooks, i personally don't use them, and IMO shouldn't be forced onto developers.

@colby-swandale colby-swandale deleted the colby/add-rubocop-into-ci branch November 17, 2018 02:17
@fatkodima
Copy link

fatkodima commented Dec 21, 2020

Lint checks on CI create a long feedback loop

For this project, rubocop will run under several seconds.

@fatkodima
Copy link

I am trying to run rubocop locally and it fails to start. So I'm guessing no one runs it under git hooks also? 😄

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

Successfully merging this pull request may close these issues.

4 participants