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

tools: overhaul tools/doc/json.js #19832

Closed
wants to merge 1 commit into from
Closed

tools: overhaul tools/doc/json.js #19832

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry for the big diff. The script seems rather old and I've decided to not create dozens of PRs for all these nits. Some changes are obvious, a few need some delving, but I can explain the unclear bits.

I just wanted that someone after me would scratch their head over this script not so long)

Here is a concise summary of all changes.

Modernize:
  • Replace var with const / let.
  • Wrap switch cases with const/let in blocks.
  • Replace common functions with arrow functions.
  • Replace string concatenation with template literals.
  • Shorthand object literals.
  • Use destructuring and spread.
Optimize:
  • Move RegExp declaration out of loops.
  • Replace .match() with .test() in boolean context.
  • Replace RegExp with string when string suffices.
  • Make RegExp more strict to reject unrelated cases.
  • Make RegExp do the trimming to eliminate many .trim() calls.
  • Cache retrieved object properties.
  • Remove conditions that cannot be false.
  • Remove code that seems obsolete (it means a state that cannot happen or is not typical).
Clarify:
  • Sync code examples in comments with the actual source state.
  • Expand some one-letter variable names.
  • Rename confusingly similar variables.
  • Move variable declarations closer to their context.
  • Remove non-actual commented out code.
  • Unify blank lines between top-level blocks.
Fix:
  • Fix conditions that cannot be true.
Guard:
  • Throw on unexpected state more often.

Modernize:
* Replace `var` with `const` / `let`.
* Wrap `switch` cases with `const`/`let` in blocks.
* Replace common functions with arrow functions.
* Replace string concatenation with template literals.
* Shorthand object literals.
* Use destructuring and spread.

Optimize:
* Move RegExp declaration out of loops.
* Replace `.match()` with `.test()` in boolean context.
* Replace RegExp with string when string suffices.
* Make RegExp more strict to reject unrelated cases.
* Make RegExp do the trimming to eliminate many `.trim()` calls.
* Cache retrieved object properties.
* Remove conditions that cannot be false.
* Remove code that seems obsolete
  (it means a state that cannot happen or is not typical).

Clarify:
* Sync code examples in comments with the actual source state.
* Expand some one-letter variable names.
* Rename confusingly similar variables.
* Move variable declarations closer to their context.
* Remove non-actual commented out code.
* Unify blank lines between top-level blocks.

Fix:
* Fix conditions that cannot be true.

Guard:
* Throw on unexpected state more often.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Apr 5, 2018
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Apr 5, 2018

@vsemozhetbyt vsemozhetbyt added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 5, 2018
vsemozhetbyt added a commit that referenced this pull request Apr 8, 2018
Modernize:
* Replace `var` with `const` / `let`.
* Wrap `switch` cases with `const`/`let` in blocks.
* Replace common functions with arrow functions.
* Replace string concatenation with template literals.
* Shorthand object literals.
* Use destructuring and spread.

Optimize:
* Move RegExp declaration out of loops.
* Replace `.match()` with `.test()` in boolean context.
* Replace RegExp with string when string suffices.
* Make RegExp more strict to reject unrelated cases.
* Make RegExp do the trimming to eliminate many `.trim()` calls.
* Cache retrieved object properties.
* Remove conditions that cannot be false.
* Remove code that seems obsolete
  (it means a state that cannot happen or is not typical).

Clarify:
* Sync code examples in comments with the actual source state.
* Expand some one-letter variable names.
* Rename confusingly similar variables.
* Move variable declarations closer to their context.
* Remove non-actual commented out code.
* Unify blank lines between top-level blocks.

Fix:
* Fix conditions that cannot be true.

Guard:
* Throw on unexpected state more often.

PR-URL: #19832
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
@vsemozhetbyt
Copy link
Contributor Author

Landed in 9a6dd07

@vsemozhetbyt vsemozhetbyt deleted the tools-doc-json branch April 8, 2018 14:00
targos pushed a commit that referenced this pull request Apr 12, 2018
Modernize:
* Replace `var` with `const` / `let`.
* Wrap `switch` cases with `const`/`let` in blocks.
* Replace common functions with arrow functions.
* Replace string concatenation with template literals.
* Shorthand object literals.
* Use destructuring and spread.

Optimize:
* Move RegExp declaration out of loops.
* Replace `.match()` with `.test()` in boolean context.
* Replace RegExp with string when string suffices.
* Make RegExp more strict to reject unrelated cases.
* Make RegExp do the trimming to eliminate many `.trim()` calls.
* Cache retrieved object properties.
* Remove conditions that cannot be false.
* Remove code that seems obsolete
  (it means a state that cannot happen or is not typical).

Clarify:
* Sync code examples in comments with the actual source state.
* Expand some one-letter variable names.
* Rename confusingly similar variables.
* Move variable declarations closer to their context.
* Remove non-actual commented out code.
* Unify blank lines between top-level blocks.

Fix:
* Fix conditions that cannot be true.

Guard:
* Throw on unexpected state more often.

PR-URL: #19832
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants