Add Prettier for linting frontend code #2493

wchargin · 2019-08-03T22:37:12Z

Fixes #2466. See commit messages for discussion:

configuration choices: 01ca437
script to generate reformatting commits: c0565d9

googlebot · 2019-08-03T22:39:19Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-08-03T22:39:52Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

wchargin · 2019-08-03T22:39:54Z

CLAs are fine because I wrote or generated all of these commits.

The author is intentionally tensorboard-gardener@google.com.

googlebot · 2019-08-03T22:53:42Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

wchargin · 2019-08-04T00:00:29Z

Travis failures are unrelated: the tf-nightly builds are installing
versions of tf-nightly from 2019-07-30 (???), which was the one
nightly that was switched over to the V2 APIs (edit: actually,
that was 2019-07-31; the 2019-07-30 package is just broken, in that you
can’t import tensorflow_estimator), and so tests are failing.

No idea why Travis is trying to install those old versions; I’ll clear
the build caches and retry.

(By this point in the build, Prettier has already passed.)

wchargin · 2019-08-04T01:43:03Z

Tests pass after cherry-picking #2494.

stephanwlee

Could not look at all files so instead sampled a few. Changes look mostly perfect except a few.

stephanwlee · 2019-08-05T16:06:36Z

tensorboard/components/tf_backend/requestManager.ts

Is it impossible to get trailing comma here too?
Rationale is the same as other trailing commas: I want git-diff to be minimal when adding/removing arguments.

I wish, but our version of TypeScript doesn’t support that:

ERROR: ${PWD}/tensorboard/plugins/histogram/tf_histogram_dashboard/BUILD:7:1: Compiling 34 TypeScript files //tensorboard/plugins/histogram/tf_histogram_dashboard:tf_histogram_dashboard failed (Exit 1) execrooter failed: error executing command bazel-out/host/bin/tensorboard/scripts/execrooter bazel-out/k8-fastbuild/bin/tensorboard/plugins/histogram/tf_histogram_dashboard/tf_histogram_dashboard-tsc-execroot.json ... (remaining 16 argument(s) skipped) Use --sandbox_debug to see verbose messages from the sandbox ../../../../../../../../../../../../../../tmp/tmp_VP6Gz/tf-histogram-dashboard/histogramCore.ts(23,11): error TS1009: Trailing comma not allowed. ../../../../../../../../../../../../../../tmp/tmp_VP6Gz/tf-histogram-dashboard/histogramCore.ts(29,26): error TS1009: Trailing comma not allowed.

See:

TypeScript "trailing comma not allowed" error in type parameters prettier/prettier#1820

Allow trailing commas in type parameter/argument lists microsoft/TypeScript#16152

If at some point we upgrade our toolchains, we can flip this flag and
re-Prettify; the diff is comparatively small (1800 lines).

Wow, didn't realize our version of tsc didn't support this. Okay, LGTM.

tensorboard/components/tf_dashboard_common/dashboard-style.html

googlebot · 2019-08-05T16:30:53Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

stephanwlee

💅!

stephanwlee

(remove the [testing...] bit from the PR title if you are ready for submission :) )

Summary: Justifications: - We use `singleQuote: true` is for consistency with Google’s JavaScript style guide. - We don’t use `trailingComma: "all"` because that’s incompatible with some compilers in our toolchain; `trailingComma: "es5"` gets us most of the way there. - Other options are due to prevalence/preference. Test Plan: Running Prettier on the `.prettierrc.json` file itself succeeds, so the file is syntactically and semantically valid. wchargin-branch: prettierrc

Summary: Pinned to an exact version because Prettier’s semver does not extend to stylistic changes (by design). Test Plan: Running `yarn && yarn prettier --version` now prints `1.18.2`. wchargin-branch: deps-prettier

Test Plan: Running `yarn lint` currently fails with a long list of bad files and a nice, human-readable message. Running `yarn fix-lint` effects a large diff, after which `yarn lint` passes. wchargin-branch: package-prettier

wchargin · 2019-08-05T16:39:44Z

remove the [testing...] bit

This will be rebased, but sure. The rebase will also likely get clabot
to complain again; I’ll take care of that when it happens. :-)

googlebot · 2019-08-05T16:41:42Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Summary: The `--ignore-engines` appears to be needed on Travis (but not locally); without it, Angular cannot be installed: ``` error @angular-devkit/architect@0.800.6: The engine "node" is incompatible with this module. Expected version ">= 10.9.0". error Found incompatible module ``` The preceding large sequence of change commits was generated with: ``` #!/bin/sh set -eu yarn fix-lint export GIT_AUTHOR_NAME="TensorBoard Gardener" export GIT_AUTHOR_EMAIL="tensorboard-gardener@google.com" # Commit each directory in order of longest-named directory to # shortest-named, to force a topological ordering. Collapse test and # demo directories, and group by parent directory to reduce from ~110 # commits to about ~20. git diff --numstat \ | awk '{ print $3 }' \ | rev \ | cut -d / -f 2- \ | rev \ | sed -e 's#/$test\|demo$$##' \ | xargs -n 1 dirname \ | sort -u \ | awk '{ print length($0), $0 }' \ | sort -nrs \ | awk '{ print $2 }' \ | while read -r dir do git add "${dir}" if git commit -m "prettier: reformat directory ${dir}" >/dev/null 2>&1; then printf 'reformat %s: ' "${dir}" git show --oneline --shortstat | tail -n 1 else printf 'skip %s (covered by subtrees)\n' "${dir}" fi done ``` Test Plan: Running `yarn lint` currently passes, as all files have been fixed up. It takes about 11 seconds on my machine. wchargin-branch: ci-prettier

wchargin · 2019-08-05T20:02:02Z

Were you thinking we'd rebase-and-merge this versus squash-and-merge?

Yes, per discussion.

is there some way we could dial this into the range of say <20
somewhat larger commits

Sure; we can group by parent directory to reduce to 21 commits, the
largest of which has a 17036 insertions. Viewing a commit of this size
in vim-fugitive is moderately slow (about 1 second), but much faster
than (e.g.) the initial TensorFlow commit.

Breakdown of --shortstats

reformat tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js:  4 files changed, 111 insertions(+), 98 deletions(-)
skip tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter (covered by subtrees)
reformat tensorboard/plugins/example/tensorboard_plugin_example:  1 file changed, 17 insertions(+), 17 deletions(-)
reformat tensorboard/plugins/interactive_inference:  15 files changed, 7437 insertions(+), 5048 deletions(-)
reformat tensorboard/plugins/profile/memory_viewer:  15 files changed, 1108 insertions(+), 960 deletions(-)
reformat tensorboard/plugins/profile/pod_viewer:  8 files changed, 1350 insertions(+), 1150 deletions(-)
reformat tensorboard/plugins/custom_scalar:  5 files changed, 716 insertions(+), 600 deletions(-)
reformat tensorboard/plugins/distribution:  5 files changed, 757 insertions(+), 270 deletions(-)
reformat tensorboard/plugins/histogram:  8 files changed, 1784 insertions(+), 744 deletions(-)
reformat tensorboard/plugins/projector:  57 files changed, 11616 insertions(+), 10256 deletions(-)
reformat tensorboard/plugins/beholder:  3 files changed, 436 insertions(+), 401 deletions(-)
reformat tensorboard/plugins/debugger:  14 files changed, 3154 insertions(+), 2678 deletions(-)
reformat tensorboard/plugins/pr_curve:  3 files changed, 371 insertions(+), 311 deletions(-)
reformat tensorboard/plugins/hparams:  26 files changed, 3863 insertions(+), 3215 deletions(-)
reformat tensorboard/plugins/profile:  12 files changed, 3089 insertions(+), 2593 deletions(-)
reformat tensorboard/plugins/scalar:  3 files changed, 303 insertions(+), 251 deletions(-)
reformat tensorboard/plugins/audio:  3 files changed, 115 insertions(+), 101 deletions(-)
reformat tensorboard/plugins/graph:  58 files changed, 15876 insertions(+), 14003 deletions(-)
reformat tensorboard/plugins/image:  2 files changed, 137 insertions(+), 94 deletions(-)
reformat tensorboard/plugins/mesh:  3 files changed, 409 insertions(+), 343 deletions(-)
reformat tensorboard/plugins/text:  2 files changed, 67 insertions(+), 46 deletions(-)
reformat tensorboard/components:  122 files changed, 17036 insertions(+), 10875 deletions(-)
skip tensorboard/plugins (covered by subtrees)
skip tensorboard (covered by subtrees)

googlebot · 2019-08-05T20:02:22Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

wchargin · 2019-08-05T20:02:31Z

CLAs are fine because I wrote or generated all of these commits.

The author is intentionally tensorboard-gardener@google.com.

Summary: Since #2472, this is needed to alter frontend dependencies; since #2493, it is needed to lint frontend code. Test Plan: Ask a guinea pig to follow these instructions. wchargin-branch: docs-yarn

Summary: Since #2472, this is needed to alter frontend dependencies; since #2493, it is needed to lint frontend code. wchargin-branch: docs-yarn

wchargin added core:frontend type:build/install labels Aug 3, 2019

googlebot added the cla: no label Aug 3, 2019

wchargin added cla: yes and removed cla: no labels Aug 3, 2019

wchargin force-pushed the wchargin-prettier branch from 6d79959 to 2d291e4 Compare August 3, 2019 22:51

googlebot added cla: no and removed cla: yes labels Aug 3, 2019

wchargin force-pushed the wchargin-prettier branch from 2d291e4 to ac2df34 Compare August 4, 2019 00:25

wchargin requested a review from stephanwlee August 4, 2019 01:43

stephanwlee reviewed Aug 5, 2019

View reviewed changes

stephanwlee added cla: yes and removed cla: no labels Aug 5, 2019

stephanwlee approved these changes Aug 5, 2019

View reviewed changes

stephanwlee reviewed Aug 5, 2019

View reviewed changes

wchargin added 3 commits August 5, 2019 09:33

package: add prettier pinned to 1.18.2

6262972

Summary: Pinned to an exact version because Prettier’s semver does not extend to stylistic changes (by design). Test Plan: Running `yarn && yarn prettier --version` now prints `1.18.2`. wchargin-branch: deps-prettier

package: add scripts to check and apply Prettier

f172cc5

Test Plan: Running `yarn lint` currently fails with a long list of bad files and a nice, human-readable message. Running `yarn fix-lint` effects a large diff, after which `yarn lint` passes. wchargin-branch: package-prettier

wchargin force-pushed the wchargin-prettier branch from ac2df34 to b4dd900 Compare August 5, 2019 16:39

googlebot added cla: no and removed cla: yes labels Aug 5, 2019

tensorboard-gardener and others added 18 commits August 5, 2019 11:54

prettier: reformat directory tensorboard/plugins/profile/pod_viewer

d1398b5

prettier: reformat directory tensorboard/plugins/custom_scalar

9e2f802

prettier: reformat directory tensorboard/plugins/distribution

50faca8

prettier: reformat directory tensorboard/plugins/histogram

76f652f

prettier: reformat directory tensorboard/plugins/projector

a0d39d3

prettier: reformat directory tensorboard/plugins/beholder

9010c93

prettier: reformat directory tensorboard/plugins/debugger

a63a53b

prettier: reformat directory tensorboard/plugins/pr_curve

d6d9624

prettier: reformat directory tensorboard/plugins/hparams

fcf75ef

prettier: reformat directory tensorboard/plugins/profile

cbf6a3b

prettier: reformat directory tensorboard/plugins/scalar

0686712

prettier: reformat directory tensorboard/plugins/audio

181578e

prettier: reformat directory tensorboard/plugins/graph

089f7a6

prettier: reformat directory tensorboard/plugins/image

3e15b1c

prettier: reformat directory tensorboard/plugins/mesh

7368900

prettier: reformat directory tensorboard/plugins/text

e74318a

prettier: reformat directory tensorboard/components

09718fa

wchargin force-pushed the wchargin-prettier branch from b4dd900 to 3c715b6 Compare August 5, 2019 19:18

wchargin added cla: yes and removed cla: no labels Aug 5, 2019

wchargin merged commit c0565d9 into tensorflow:master Aug 5, 2019

wchargin deleted the wchargin-prettier branch August 5, 2019 20:04

wchargin mentioned this pull request Aug 5, 2019

Add Prettier [demo] #2473

Closed

wchargin mentioned this pull request Aug 5, 2019

docs: point to Yarn install instructions #2501

Merged

wchargin added a commit that referenced this pull request Aug 5, 2019

docs: point to Yarn install instructions (#2501)

757628a

Summary: Since #2472, this is needed to alter frontend dependencies; since #2493, it is needed to lint frontend code. wchargin-branch: docs-yarn

Add Prettier for linting frontend code #2493

Add Prettier for linting frontend code #2493

Uh oh!

Conversation

wchargin commented Aug 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Aug 3, 2019

Uh oh!

googlebot commented Aug 3, 2019

Uh oh!

wchargin commented Aug 3, 2019

Uh oh!

googlebot commented Aug 3, 2019

Uh oh!

wchargin commented Aug 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wchargin commented Aug 4, 2019

Uh oh!

stephanwlee left a comment

Choose a reason for hiding this comment

Uh oh!

stephanwlee Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

wchargin Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

stephanwlee Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

googlebot commented Aug 5, 2019

Uh oh!

stephanwlee left a comment

Choose a reason for hiding this comment

Uh oh!

stephanwlee left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wchargin commented Aug 5, 2019

Uh oh!

googlebot commented Aug 5, 2019

Uh oh!

wchargin commented Aug 5, 2019

Uh oh!

googlebot commented Aug 5, 2019

Uh oh!

wchargin commented Aug 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wchargin commented Aug 3, 2019 •

edited

Loading

wchargin commented Aug 4, 2019 •

edited

Loading

stephanwlee left a comment •

edited

Loading