-
Notifications
You must be signed in to change notification settings - Fork 12
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 prettier to linting configuration #154
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jackson-dean - You also need to add yarn eslint .
to .travis.yml
or linting isn't enforced in CI.
ahh, i thought this is covered by |
and which is already in travis config. I can change it though, just want to make sure its not running twice @rwjblue from the logs for the last CI run for this PR https://travis-ci.org/nickiaconis/ember-prefetch/jobs/589146899#L502 |
Hmm, you are definitely correct. I guess I assumed that this wasn't running in CI because I assumed there would be TONS of changes to apply prettier to the codebase. But I think that whats happening here is that you've added the packages for prettier, but not added it to the eslint config. Is that intentional? |
Ahh it was just an oversight. I looked at a PR in ember engines that was
bumping linting packages and just grabbed what looked relevant. Didn’t
really think too much about prettier 😅
However our time line for the project that depends on these prefetch
changes just got delayed so not a huge rush for us on this stuff. I can
look at adding prettier configs here if you would like to have it set up
and then circle back to the lazy route thing.
…On Fri, Sep 27, 2019 at 6:48 AM Robert Jackson ***@***.***> wrote:
Hmm, you are definitely correct. I guess I assumed that this wasn't
running in CI because I assumed there would be TONS of changes to apply
prettier to the codebase.
But I think that whats happening here is that you've added the packages
for prettier, but not added it to the eslint config. Is that intentional?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#154?email_source=notifications&email_token=ABYEGZ34DMUOBV5RCQ65VALQLYFJTA5CNFSM4I2ERKKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7Y6NRQ#issuecomment-535946950>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABYEGZ5X7X3UOHMW2HNZM4LQLYFJTANCNFSM4I2ERKKA>
.
|
ya, definitely love to use prettier here... |
Cool I will work tomorrow on it early next week
…On Fri, Sep 27, 2019 at 12:39 PM Robert Jackson ***@***.***> wrote:
I can look at adding prettier configs here if you would like to have it
set up
ya, definitely love to use prettier here...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#154?email_source=notifications&email_token=ABYEGZYUF4DJGDFFQNJMLDTQLZOQBA5CNFSM4I2ERKKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7Z43JI#issuecomment-536071589>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABYEGZYGQJMWPIMML7XOIRDQLZOQBANCNFSM4I2ERKKA>
.
|
- Remove ember-cli-eslint dependency and replace it with direct dependency on eslint and required plugins. NOTE: disabling no-new-mixins for since prefetch currently relies on a mixin.
8bf9ab8
to
01d4db6
Compare
I added a separate commit for running prettier. Can make it a separate PR for adding the prettier config and running prettier if that is preferred. |
npx prettier --write '**/*.js'
01d4db6
to
5c2ae5e
Compare
dependency on eslint and required plugins.
NOTE: disabling no-new-mixins for since prefetch currently relies on
a mixin.