Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Update Packages, Use Chrome to test, User Prettier to Format #25

Conversation

ChristianMurphy
Copy link
Contributor

Updates all the dependencies package.json.
Replace Phantom JS with Chrome/Puppeteer.
Use ESLint Prettier to apply Prettier formatting and ESLint analysis together.

@ChristianMurphy
Copy link
Contributor Author

@ChristianMurphy
Copy link
Contributor Author

/reviewers @davidmsibley @vertein @thevoiceofzeke @apetro

Copy link
Contributor

@vertein vertein left a comment

Choose a reason for hiding this comment

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

I'm still uneasy about things that are auto-formatting code built into the process. I would be in favor of having the script available, but not as part of the commit process.

@ChristianMurphy
Copy link
Contributor Author

ChristianMurphy commented Oct 31, 2017

I'm still uneasy about things that are auto-formatting code built into the process.

I'd encourage giving it a try.

Take a look at the format commit 0042254.
The code output is arguably very readable and an improvement over the existing code style, code that is written by competent, experienced, and intelligent people.
A tool like prettier can provide that highly readable code experience not just to experienced developers, but to anyone who contributes to the project, with no friction of a broken build because of code style.

I would be in favor of having the script available, but not as part of the commit process.

the npm run format command is included in this PR.
However relying on developers to manually run a formatter has proved problematic in the past.
uPortal-home's build has been broken for 4 days and counting because of a code style issue.

Does this automatically re-format to use Google's code style or does this use a separate style that we would have to match to Google's?

uPortal-Attic/uportal-app-framework#580 (comment)

This version will exactly follow Google code style.
It passes code through prettier to handle graceful line breaks, then into the official Google ESLint formatter for additional tuning.

I think we can be way more open to experimental friction and risk in modules that we're less likely to need to release frequently for adoption by other teams.
widget-creator is much lower risk, in that its audience is developers.

uPortal-Attic/uportal-app-framework#580 (comment)

Widget creator was suggested as a proving ground for the uPortal-app-framework team to be able to try this process out and see if its a good fit for the team dynamic.

edit corrected typos
edit 2 uPortal-home build has been fixed

Copy link
Member

@apetro apetro left a comment

Choose a reason for hiding this comment

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

Let's try it.

@ChristianMurphy
Copy link
Contributor Author

🔔 I don't have write access to this repo, if/when the team decides to merge this, someone other than me would need to click the merged button.

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.

4 participants