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 stylelint to pre-commit hook #521

Closed
wants to merge 2 commits into from
Closed

Add stylelint to pre-commit hook #521

wants to merge 2 commits into from

Conversation

brianbancroft
Copy link

@brianbancroft brianbancroft commented Nov 13, 2017

The following is a basic implementation of StyleLint on the pre-commit hook. It's not quite ready for prime-time as I have two areas I still haven't figured out:

  • Testing. I'm still stuck on how to test it
  • Unexpected output error (see screencap 1) (regex. Still looking at it)

Screencaps

Bad CSS

Below in another repo, I added the gem locally, and added some gobbedly-gook in the CSS. This is the output:

image

Good CSS

Here's what it looks like otherwise:
image

Closes #514 if resolved

Copy link
Owner

@sds sds left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, @brianbancroft.

Overall this looks fine, but as you mentioned it's missing tests. Good tests for hooks simply ensure that the branches of the hook's logic are executed. In your case, you want to check what happens when:

  • The stylelint command returns a zero exit status
  • The stylelint command returns a non-zero exit status
  • The stylelint command prints messages (i.e. is your regex working and correctly attributing file name and line number?)

See https://github.com/brigade/overcommit/blob/master/spec/overcommit/hook/pre_commit/scss_lint_spec.rb for an example. Happy to merge once tests are in place. Thanks!

@@ -2,5 +2,5 @@

# Defines the gem version.
module Overcommit
VERSION = '0.41.0'.freeze
VERSION = '0.42.0'.freeze
Copy link
Owner

Choose a reason for hiding this comment

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

Let's leave this out for now. We'll update the version when we release the next version. Thanks!

@sds
Copy link
Owner

sds commented Jul 30, 2018

Finished in #590

@sds sds closed this Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for StyleLint in Pre-Commit Hook.
2 participants