-
Notifications
You must be signed in to change notification settings - Fork 208
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
chore: use prettier for local formatting #237
Conversation
Codecov Report
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
+ Coverage 98.52% 98.53% +<.01%
==========================================
Files 12 12
Lines 814 819 +5
Branches 65 68 +3
==========================================
+ Hits 802 807 +5
Misses 12 12
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine. I'll get used to it. I would be interested in thoughts/objections from @google/google-node-team before this lands.
package.json
Outdated
@@ -24,7 +24,7 @@ | |||
"codecov": "nyc report --reporter=json && codecov -f coverage/*.json", | |||
"compile": "tsc -p .", | |||
"format-check": "./bin/format-check.sh", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be just prettier --list-different glob
now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it can! Good call out 👍
`${chalk.red('ERROR:')} ${chalk.gray('compilerOptions.outDir')} ` + | ||
`cannot use the value '.'. That would delete all of our sources.`); | ||
`${chalk.red('ERROR:')} ${chalk.gray('compilerOptions.outDir')} ` + | ||
`cannot use the value '.'. That would delete all of our sources.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh.
I guess I will get used to it.
options: Options, | ||
files: string[], | ||
fix?: boolean | ||
) => Promise<boolean>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sigh.
I guess I will get used to it.
const BASE_ARGS_INLINE = [ | ||
'-style', | ||
'{Language: JavaScript, BasedOnStyle: Google, ColumnLimit: 80}', | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whatevs.
path.join(options.targetRootDir, '.clang-format') | ||
) | ||
? BASE_ARGS_FILE | ||
: BASE_ARGS_INLINE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😿
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, AFAIS I like this formatting pattern better.
Hooray! This has been the only blocker for us to adopt |
This is the first step towards a prettier migration. Adding the dependency, and using it to format our source only as a first step.