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

chore(package.json): trigger lint against staged files only #1797

Merged
merged 1 commit into from
Jul 9, 2016

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Jul 1, 2016

Description:
This PR introduces module lint-staged via wiring existing git hooks to pre-commit. Since checked in codebase is already guarded by full lint, it doesn't necessarily required to trigger full lint each time per check in. This PR makes automatically trigger lint only against staged files, reduced daily-flow time consumption on lint.

  • Per each commit, lint will be triggered automatically only for staged files. Failure will block commit.
  • All other lint tasks remain, allows manual full lint if possible.
  • lint_staged task will allow trigger lint manually against staged files, do not need try commit each time for trigger lint.
  • travis will run full lint per each build to serve as guard.

NOTE : I think #1731 is still valid when codebase grown, need frequent full lint including CI servers.

Related issue (if exists):

relates to #1731.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 1, 2016

/cc @Blesh suffered by slow lint. While this isn't definite answer to lint speed itself, this can be short-term workaround.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.472% when pulling 9231c21 on kwonoj:lint-staged into 5158f27 on ReactiveX:master.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 7, 2016

LGTM

@kwonoj
Copy link
Member Author

kwonoj commented Jul 7, 2016

above comment was trying to see if I can pass LGTM checker. Seems not.

@kwonoj kwonoj mentioned this pull request Jul 8, 2016
@staltz
Copy link
Member

staltz commented Jul 8, 2016

LGTM

@staltz
Copy link
Member

staltz commented Jul 8, 2016

I think LGTM.co needs at least two confirmations.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 8, 2016

I think LGTM.co needs at least two confirmations.

Make sense, let me try later.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.472% when pulling 26e6c19 on kwonoj:lint-staged into 2aa1433 on ReactiveX:master.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 8, 2016

LGTM

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.472% when pulling d417ab4 on kwonoj:lint-staged into 22fd8ae on ReactiveX:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.472% when pulling ebf3210 on kwonoj:lint-staged into 8befc1e on ReactiveX:master.

@kwonoj
Copy link
Member Author

kwonoj commented Jul 8, 2016

as I've verified this on windows machine only yet, I'll leave this opened and ask opinion to @staltz , @Blesh and any other lives on non-windows system.

@coveralls
Copy link

coveralls commented Jul 9, 2016

Coverage Status

Coverage remained the same at 96.472% when pulling 4b290ff on kwonoj:lint-staged into 0fc88b2 on ReactiveX:master.

@jayphelps
Copy link
Member

LGTM

@jayphelps jayphelps merged commit 3632489 into ReactiveX:master Jul 9, 2016
@kwonoj kwonoj deleted the lint-staged branch October 4, 2016 03:34
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants