Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

chore: Run prettier/tslint in ci#97

Merged
sagirk merged 3 commits into
nodejs:masterfrom
ollelauribostrom:prettier-travis
Mar 8, 2019
Merged

chore: Run prettier/tslint in ci#97
sagirk merged 3 commits into
nodejs:masterfrom
ollelauribostrom:prettier-travis

Conversation

@ollelauribostrom

@ollelauribostrom ollelauribostrom commented Feb 21, 2019

Copy link
Copy Markdown
Contributor

Summary of changes:

  • Bump prettier version to 1.16.4
  • Add format-check and tslint scripts to package.json
  • Run format-check and tslint scripts in Travis CI

Note:

Running yarn format-check will currently fail with exit code 1 since a bunch of files aren't formatted correctly (this is what's causing the build for this PR to fail). Once we have decided on some formatting rules (#90) and the project is formatted in a unform way, this will work as a guard for not merging code that isn't correctly formatted or that is missing type annotations etc.

Comment thread .travis.yml Outdated
@ollelauribostrom ollelauribostrom force-pushed the prettier-travis branch 3 times, most recently from e2379ac to c15d74b Compare February 23, 2019 00:11
@ollelauribostrom

Copy link
Copy Markdown
Contributor Author

Since #90 got merged, I ran prettier here to format all of the files according to the new set of rules 💅

@LaRuaNa

LaRuaNa commented Feb 24, 2019

Copy link
Copy Markdown
Contributor

@ollelauribostrom We definitely need that but I'd like to wait until the next meeting so we all can agree on the current prettierrc. Would like to avoid clutter up git history with syntax back and forth.

@sagirk

sagirk commented Mar 1, 2019

Copy link
Copy Markdown
Contributor

@ollelauribostrom This looks good. Can we rebase against upstream and run prettier again?

The prettier rules were recently updated in #153.

@ollelauribostrom

Copy link
Copy Markdown
Contributor Author

@sagirk I'm on it! 🙂

@LaRuaNa

LaRuaNa commented Mar 1, 2019

Copy link
Copy Markdown
Contributor

@sagirk @ollelauribostrom Can we please wait with that for a while. I'll open a big refactoring PR would be quite painful to resolve syntax conflicts :[

@ollelauribostrom

Copy link
Copy Markdown
Contributor Author

@LaRuaNa Yep sure sounds like a good idea 👍

@sagirk

sagirk commented Mar 7, 2019

Copy link
Copy Markdown
Contributor

@ollelauribostrom #157 is finally merged. 🎉
Let's rebase against master and run prettier/tslint again.

@ollelauribostrom

ollelauribostrom commented Mar 7, 2019

Copy link
Copy Markdown
Contributor Author

@sagirk This should now be up to speed with master. Not sure why the travis build fails, everything works as expected locally 🤔 Any ideas?

Comment thread package.json
@LaRuaNa

LaRuaNa commented Mar 8, 2019

Copy link
Copy Markdown
Contributor

Just to tidy up a bit and prevent double work: Here an issue https://github.com/nodejs/nodejs.dev/issues/164 about prettier hooks. And I'm working on linter / prettier integration with industry standards like airbnb config as decided in the last meeting

@ZYSzys

ZYSzys commented Mar 8, 2019

Copy link
Copy Markdown
Member

Just make a small change to fix prettier @ollelauribostrom

@MylesBorins @LaRuaNa PTAL.
I think this looks good at least now.

(BTW, we are still ok to wait with your big refactoring PR if needed :)

@sagirk sagirk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! 🎉

Thanks @ollelauribostrom! ❤️

@sagirk

sagirk commented Mar 8, 2019

Copy link
Copy Markdown
Contributor

Landing this now.

https://github.com/nodejs/nodejs.dev/issues/164 can follow after this.

#179 might need some more time to be fully fleshed out.

@sagirk sagirk merged commit d5795ce into nodejs:master Mar 8, 2019
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