Design Review 2019-03-06 21:00 UTC (prettier for code formatting)

[mrjoro]

Time: 2019-03-06 21:00 UTC (add to Google Calendar)
Location: Video conference via Google Meet

The AMP Project holds weekly engineering design reviews. We encourage everyone in the community to participate in these design reviews.

If you are interested in bringing your design to design review, read the design review documentation and add a link to your design doc by the Monday before your design review.

When attending a design review please read through the designs before the design review starts. This allows us to spend more time on discussion of the design.

We rotate our design review between times that work better for different parts of the world as described in our design review documentation, but you are welcome to attend any design review. If you cannot make any of the design reviews but have a design to discuss please let mrjoro@ know on Slack and we will find a time that works for you.

[nainar]

Potential topic: #20237

[rsimha]

Adopt prettier for code formatting: #17086 and #21212

[nainar]

Taking the loaders topic of the agenda, since we don't have the approvals on the designs yet.

[aghassemi]

Adopt prettier for code formatting: #17086 and #21212

• Partially replace eslint with prettier which will auto format code automatically
• We do lots of nits related to formatting during code review
• We also do lots of lint commits after the fact
• Adopting prettier will remove the need for the 2 points mentioned above.
• Currently AMP uses eslint for both formatting and code correctness (e.g. unused variables, custom rules)
• Proposal: We keep using eslint for code correctness but we use prettier for formatting.
• @sparhami : Did you explore clag format, do you know how it compares to prettier?
• @rsimha: We haven't played with clag format, we went with prettier because it is common in JS and it is OSS.
• @erwinmombay We tried it in https://github.com/ampproject/amphtml/pull/774/files
• @cramforce clag format is not as pretty and does not have idiomatic JS formatting.
• @torch2424 I like to bring up points raised and maybe prettier is not the best for us.
• @alanorozco it may change template literals, @jridgewell it must be a setting we can configure since it is a semantic change when it does it on template literals.
• Next Steps:
• Outstanding bug that prettier is fixing (automatic removal of quotes around object field names_
• We need to do one run across all code considering directives currently used for ignoring certain eslints at file and line level
• Test our code
• Merging this change will be fun!
• @jridgewell if we breakup into multiple folders, it can help with merge. @rsimha I will probably do this late Friday to avoid conflict.
• @jridgewell people can avoid conflict to rejecting prettier changes and running a lint --fix after. We can automate this for others.
• @alanorozco For comments that are inline, we should move them to line above before formatting
• @sparhami Does this impact /*ok*/? @jridgewell We can change that to use eslint
• @sparhami Does this work fine with closer type cast? because it is inline (/* @type .. ). It probably works because of parentheses.
• issue Line 86 in file: 3p/messaging.js , it made the wrong parentheses for CC type.

[rsimha]

Today's presentation: AMP + Prettier.pdf