Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Formatting fixes for ESLint #991

Merged
merged 1 commit into from
Oct 17, 2015
Merged

Conversation

ilanbiala
Copy link
Member

This has all the formatting changes and ESLint support (temporarily so we can confirm it passes in Travis).

@ilanbiala ilanbiala added this to the 0.4.2 milestone Oct 15, 2015
@ilanbiala
Copy link
Member Author

@lirantal @mleanos @codydaig once you guys give me the word and are happy with this formatting PR, I'll remove ESLint support from this and merge it and then we can go back to #990 for actual ESLint support.

@codydaig
Copy link
Member

LGTM.

Also, I'm cool with combining the PRs and just squashing down to two commits (one for the eslint addition, and one for the formatting changes).

@mleanos
Copy link
Member

mleanos commented Oct 16, 2015

I'm also fine with two commits in one PR.

I'd like to take this for a quick test run, and review. I should be able to complete that in an hour or so.

@mleanos
Copy link
Member

mleanos commented Oct 16, 2015

LGTM. Thanks @ilanbiala

@mleanos
Copy link
Member

mleanos commented Oct 16, 2015

@ilanbiala Upon looking at this again, I realized the Gulp task is failing.. It throws an error at this line
https://github.com/meanjs/mean/pull/991/files#diff-b9e12334e9eafd8341a6107dd98510c9R112

4:58] Starting 'eslint'...
4:58] 'eslint' errored after 478 ms
4:58] TypeError: undefined is not a function

@ilanbiala
Copy link
Member Author

@mleanos I'm thinking it stems from this line because we don't pass in a formatter and when it tries to write it can't execute the function because it's undefined https://github.com/adametry/gulp-eslint/blob/master/util.js#L180. Can you paste in the stack trace if there is one?

@ilanbiala
Copy link
Member Author

I'm going to do them as 2 separate PRs because I already have it set up like that locally. @mleanos try adding .eslint in between plugins.format() because the plugin is eslint and the method is format. Once you confirm, I'll merge this and then the actual ESLint setup in the other PR.

@ilanbiala ilanbiala self-assigned this Oct 16, 2015
@mleanos
Copy link
Member

mleanos commented Oct 16, 2015

@ilanbiala adding ".eslint" to that line fixed the Gulp issues..

This LGTM now! Thank you :)

ilanbiala added a commit that referenced this pull request Oct 17, 2015
@ilanbiala ilanbiala merged commit 52f94a2 into meanjs:master Oct 17, 2015
@ilanbiala ilanbiala deleted the formatting-fixes branch October 17, 2015 01:06
@lirantal
Copy link
Member

I missed the party but good thing to see this already fixed and joined in with the eslint file!
Great job @ilanbiala

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants