Skip to content
This repository was archived by the owner on Jul 19, 2021. It is now read-only.

Toggle linting #7

Merged
merged 5 commits into from
Aug 25, 2016
Merged

Toggle linting #7

merged 5 commits into from
Aug 25, 2016

Conversation

macdonaldr93
Copy link
Contributor

@Shopify/themes-fed

Added a boolean to toogle in config.js to enable/disable linting on build/watch. You can always run the test task to lint.

I ended combining an update to browserify in here. Watch task wasn't working very well - sometimes it wouldn't upload, other times it would upload twice - so I flipped it over to chokidar and set it to re-bundle on file change. We get to remove watchify which was the 5th slowest dependency. @m-ux and @t-kelly if you guys could review this in depth that would be great.

@macdonaldr93 macdonaldr93 self-assigned this Aug 24, 2016
@macdonaldr93
Copy link
Contributor Author

PS: I think it was this part of watchify that had some undesirable effects.

Important: Watchify will not emit 'update' events until you've called w.bundle() once and completely drained the stream it returns.

We had an extra compile on slate start and slate watch.

@@ -16,7 +17,7 @@ var messages = require('./includes/messages.js');
* @memberof slate-cli.tasks.build
* @static
*/
gulp.task('build:css', ['lint:css'], function() {
gulp.task('build:css', [].concat(lintTask), function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to just put lintTask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought was to make it more obvious that additional tasks should be added in the first array. But maybe that's overkill. Thoughts?

@cshold
Copy link
Contributor

cshold commented Aug 25, 2016

Sweeeeeet 👍

@mcpatten
Copy link
Contributor

R+ 🚀

re: watchify, might be worth opening an issue on the GH project.

@macdonaldr93 macdonaldr93 merged commit db9ff36 into master Aug 25, 2016
@macdonaldr93 macdonaldr93 deleted the toggle-linting branch August 25, 2016 15:09
@chrisberthe
Copy link
Contributor

👍

@chrisberthe
Copy link
Contributor

#toolate

t-kelly pushed a commit that referenced this pull request Jul 31, 2017
* Clone a repo to your project folder

* Changes

* changes

* Changes

* Change

* refactor some code

* install dependencies

* remove random files

* git init and replace slate pkg

* add git/yeoman tpl files/env questions

* finish todos and start commenting code

* remove extra logs

* update comment on private function

* add last private function comment

* absolute dep

* commit stuff

* get generator files from slate repo

* pull templates from cache

* fix indent
t-kelly pushed a commit that referenced this pull request Jul 31, 2017
t-kelly pushed a commit that referenced this pull request Jul 31, 2017
* Clone a repo to your project folder

* Changes

* changes

* Changes

* Change

* refactor some code

* install dependencies

* remove random files

* git init and replace slate pkg

* add git/yeoman tpl files/env questions

* finish todos and start commenting code

* remove extra logs

* update comment on private function

* add last private function comment

* absolute dep

* commit stuff

* get generator files from slate repo

* pull templates from cache

* fix indent
t-kelly pushed a commit that referenced this pull request Jul 31, 2017
t-kelly pushed a commit that referenced this pull request Jul 31, 2017
* Clone a repo to your project folder

* Changes

* changes

* Changes

* Change

* refactor some code

* install dependencies

* remove random files

* git init and replace slate pkg

* add git/yeoman tpl files/env questions

* finish todos and start commenting code

* remove extra logs

* update comment on private function

* add last private function comment

* absolute dep

* commit stuff

* get generator files from slate repo

* pull templates from cache

* fix indent
t-kelly pushed a commit that referenced this pull request Jul 31, 2017
@lock
Copy link

lock bot commented Oct 26, 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 Oct 26, 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.

5 participants