Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Handle jshinting via ember-cli's new 'lint' hooks #28

Merged
merged 1 commit into from
Jan 25, 2015

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Jan 25, 2015

No description provided.

if (type === 'lint-app' || type === 'lint-tests' || type === 'lint-addon') {
return jshintTrees(tree, {
jshintrcPath: this.jshintrc.tests,
description: 'JSHint - QUnit'
Copy link
Member

Choose a reason for hiding this comment

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

Can this be 'JSHint - ' + type (otherwise they would all be reported as JSHint - QUnit)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, these are all QUnit specific. But I could also add the type.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the point is the type. When this shows up in the slow trees log how do we know WHICH tree was slow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rwjblue
Copy link
Member

rwjblue commented Jan 25, 2015

One small nit-pick, but LGTM.

rwjblue added a commit that referenced this pull request Jan 25, 2015
Handle jshinting via ember-cli's new 'lint' hooks
@rwjblue rwjblue merged commit 56a480c into ember-cli:master Jan 25, 2015
@rwjblue
Copy link
Member

rwjblue commented Jan 25, 2015

@jakecraige - Can you release (and potentially add me to npm)?

@rwjblue
Copy link
Member

rwjblue commented Jan 25, 2015

@dgeb - Will need to be mirrored in ember-cli-mocha.

@ef4
Copy link
Contributor Author

ef4 commented Jan 25, 2015

@dgeb I have a PR, I wasn't going to bother you until ember-cli releases.

@ef4
Copy link
Contributor Author

ef4 commented Jan 25, 2015

Apologies, but I found a problem with the way I originally did this. We want linters to run in parallel, not in series, so we can't use the existing postprocessTree hook. Please take #29 to switch.

@dgeb
Copy link
Member

dgeb commented Jan 25, 2015

@ef4 should I hold off on merging ember-cli/ember-cli-mocha#26 ?

@ef4
Copy link
Contributor Author

ef4 commented Jan 25, 2015

ember-cli/ember-cli-mocha#26 is already updated to match my change above, but you may want to wait until the corresponding change lands in ember-cli:

ember-cli/ember-cli#3094

In case anybody else points out another reason the API would need to change.

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.

3 participants