Firestore: Add "max-line-length" and "ordered-imports" tslint rules. #1238
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikelehen
requested review from
bojeil-google,
depoll,
Feiyang1,
hiranya911,
rsgowman,
schmidt-sebastian,
var-const,
wilhuff and
zxu123
as code owners
gsoltis
approved these changes
Sep 21, 2018
There was a problem hiding this comment.
Definitely LGTM for the import organization stuff.
I'm less excited about the line length change than I was hoping to be. Re-wrapping comments is good. Having to split string literals and rename variables seems to me to be less obviously good. That being said, we can always // tslint:disable for cases where it makes sense, so maybe this is the right tradeoff.
|
Fei, can you approve this for the .vscode/settings.json file (https://github.com/firebase/firebase-js-sdk/pull/1238/files#diff-5b46cfda0ddaed371f52aa24ea2b0b2a)? I don't think anybody else relies on those settings (I created the file in the first place). |
Feiyang1
approved these changes
Sep 21, 2018
mikelehen
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enforces a maximum line length (of 100) and import ordering via tslint rules and fixes all violations.
The line length violations were fixed manually. Prettier already targets a line length of 80 for code (but this isn't a strict limit) so mostly I just split string literals and long comments, but I did have to rename one variable to get prettier not to exceed the line length. :-/
The import ordering violations were fixed automatically via
yarn lint:fix.If it's easier, you can review the two commits separately, but there's really no overlap.