Skip to content
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

client: autoformat all files with calypso-prettier #18282

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

samouri
Copy link
Contributor

@samouri samouri commented Sep 26, 2017

Inspired by: #14801, argued for in #12260 (comment)

Uses our custom build of prettier to auto-format existing files in the client directory.

If we are going to use prettier then I vote we use prettier. This takes all of the incidental formatting changes out of future PRs dealing with code in client. If we have concern for git history then we simply need to use the button in Github to go back to the previous revision - no problem. It's there anyway in any git history.

Generated by running

./node_modules/.bin/prettier --write client/**/*.js
./node_modules/.bin/prettier --write client/**/*.jsx

@matticbot
Copy link
Contributor

@samouri samouri force-pushed the format/auto-prettify-client branch from 8c74cf3 to a0df5ab Compare October 4, 2017 21:26
@samouri samouri changed the title Calypso-Prettier /client client: autoformat all files with calypso-prettier Oct 4, 2017
@samouri samouri self-assigned this Oct 4, 2017
@samouri samouri added Build Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Oct 4, 2017
@samouri samouri force-pushed the format/auto-prettify-client branch from a0df5ab to 4db3aa5 Compare October 5, 2017 14:17
@ockham
Copy link
Contributor

ockham commented Oct 5, 2017

image

Seems legit 😆

@samouri samouri changed the title client: autoformat all files with calypso-prettier wp-calypso: autoformat all files with calypso-prettier Oct 5, 2017
@samouri samouri changed the title wp-calypso: autoformat all files with calypso-prettier client: autoformat all files with calypso-prettier Oct 5, 2017
@hoverduck
Copy link
Contributor

I ran the e2e tests against the branch and didn't come across any problems.

@samouri samouri force-pushed the format/auto-prettify-client branch from 4db3aa5 to 12b5f44 Compare October 5, 2017 17:57
@samouri samouri merged commit b855b7a into master Oct 5, 2017
@samouri samouri deleted the format/auto-prettify-client branch October 5, 2017 18:09
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 5, 2017
@sirreal
Copy link
Member

sirreal commented Oct 6, 2017

🎉

nylen added a commit that referenced this pull request Oct 6, 2017
nylen added a commit that referenced this pull request Oct 11, 2017
@aduth
Copy link
Contributor

aduth commented Oct 24, 2017

Our selectors DevDocs parsing is pretty naive, and changes here to include @format in the "External dependencies" block is now causing it to be considered as the documentation for a selector.

https://github.com/Automattic/wp-calypso/blob/master/client/state/selectors/get-site-url.js
https://wpcalypso.wordpress.com/devdocs/selectors

We could improve selector file parsing (as previously attempted in #10487), but begs the question if we really want the @format annotation to be merged into our dependencies DocBlocks.

@samouri
Copy link
Contributor Author

samouri commented Oct 24, 2017

We could improve selector file parsing (as previously attempted in #10487)
but begs the question if we really want the format annotation to be merged into our dependencies DocBlocks.

I don't think the two items are mutually exclusive. As to the latter, I've written a codemod that can split up the format docblock and the dependencies docblock. Right now the functionality is part of sort-imports but if we want we can isolate the feature

@a8ci18n
Copy link

a8ci18n commented May 22, 2020

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/2900697

Hi @samouri, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:

  • Front Page, Archive Pages, and Search Results
  • Action required: Please contact your domain registrar to point {{strong}}%(domainName)s's{{/strong}} name server records to WordPress.com.
  • Sharing posts to your Facebook page.
  • We normally reply within 24-48 hours but are experiencing longer delays right now. We appreciate your patience and will respond as soon as we can.
  • Footer credit
  • Change your site's appearance in a few clicks, with an expanded selection of fonts and colors.

Thank you in advance!

@a8ci18n
Copy link

a8ci18n commented May 26, 2020

Translation for this Pull Request has now been finished.

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

Successfully merging this pull request may close these issues.

7 participants