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: modernize and optimize doc/addon-verify.js #20188

Closed
wants to merge 1 commit into from
Closed

tools: modernize and optimize doc/addon-verify.js #20188

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Apr 21, 2018

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

Modernize:

  • Replace var with const / let.
  • Replace common functions with arrow functions.
  • Use destructuring.
  • Use String.prototype.padStart(), String.prototype.endsWith().

Optimize:

  • Reduce function calls.
  • Reduce intermediate variables.
  • Cache retrieved object properties.
  • Move RegExp declaration out of a cycle.
  • Simplify RegExps.
  • Replace RegExp with string when string suffices.
  • Remove conditions that cannot be false.
  • Replace for..in with Object.keys().forEach().

Also, eliminate needlessly complicated function chains:

  • ondone callback only checks errors;
  • if there is an error, it is called once and throws, then script exits;
  • if there are no errors, it is noop;
  • so there is no need to wrap it into once() function;
  • and there is no need to call it without errors;
  • we can eliminate it and replace with throw where an error occurs;
  • we can also replace onprogress callback with console.log in place;
  • at last, we can eliminate waiting counter and once() utility.

The new script produces results identical to the old ones.

Modernize:
* Replace `var` with `const` / `let`.
* Replace common functions with arrow functions.
* Use destructuring.
* Use spread, eliminate `arguments`.
* Use `String.prototype.padStart()`, `String.prototype.endsWith()`.

Optimize:
* Reduce function calls.
* Reduce intermediate variables.
* Cache retrieved object properties.
* Move RegExp declaration out of a cycle.
* Simplify RegExps.
* Replace RegExp with string when string suffices.
* Remove conditions that cannot be false.
* Replace for..in with `Object.keys().forEach()`.
* Rename confusingly similar variables.

Also, eliminate needlessly complicated function chains:
* `ondone` callback only checks errors;
* if there is an error, it is called once and throws, then script exits;
* if there are no errors, it is noop;
* so there is no need to wrap it into `once()` function
* and there is no need to call it without errors;
* we can eliminate it and replace with `throw` where an error occurs;
* we can also replace `onprogress` callback with `console.log` in place;
* at last, we can eliminate `waiting` counter and `once()` utility.

The new script produces results identical to the old ones.
@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 21, 2018
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Apr 21, 2018

Full CI to be on the safe side: https://ci.nodejs.org/job/node-test-pull-request/14412/

Failures seem unrelated, but I'll rerun:
https://ci.nodejs.org/job/node-test-pull-request/14414/


const validNames = /^\/\/\s+(.*\.(?:cc|h|js))[\r\n]/;
tokens.forEach(({ type, text }) => {
if (type === 'heading') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the check for text required here?

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt Apr 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I tested correctly, there cannot be empty headings: they are not recognized as headings by marked:

const marked = require('marked ');

const md = `
# heading

text

#

text
`;

console.log(marked.lexer(md));
[ { type: 'heading', depth: 1, text: 'heading' },
  { type: 'paragraph', text: 'text' },
  { type: 'paragraph', text: '#' },
  { type: 'paragraph', text: 'text' },
  links: {} ]

@vsemozhetbyt
Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 22, 2018
@vsemozhetbyt
Copy link
Contributor Author

Landed in 6946812

vsemozhetbyt added a commit that referenced this pull request Apr 24, 2018
Modernize:
* Replace `var` with `const` / `let`.
* Replace common functions with arrow functions.
* Use destructuring.
* Use `String.prototype.padStart()`, `String.prototype.endsWith()`.

Optimize:
* Reduce function calls.
* Reduce intermediate variables.
* Cache retrieved object properties.
* Move RegExp declaration out of a cycle.
* Simplify RegExps.
* Replace RegExp with string when string suffices.
* Remove conditions that cannot be false.
* Replace for..in with `Object.keys().forEach()`.

Also, eliminate needlessly complicated function chains:
* `ondone` callback only checks errors;
* if there is an error, it is called once and throws, then script exits;
* if there are no errors, it is noop;
* so there is no need to wrap it into `once()` function
* and there is no need to call it without errors;
* we can eliminate it and replace with `throw` where an error occurs;
* we can also replace `onprogress` callback with `console.log` in place;
* at last, we can eliminate `waiting` counter and `once()` utility.

The new script produces results identical to the old ones.

PR-URL: #20188
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@vsemozhetbyt vsemozhetbyt deleted the tools-doc-addon-verify branch April 24, 2018 07:40
MylesBorins pushed a commit that referenced this pull request May 4, 2018
Modernize:
* Replace `var` with `const` / `let`.
* Replace common functions with arrow functions.
* Use destructuring.
* Use `String.prototype.padStart()`, `String.prototype.endsWith()`.

Optimize:
* Reduce function calls.
* Reduce intermediate variables.
* Cache retrieved object properties.
* Move RegExp declaration out of a cycle.
* Simplify RegExps.
* Replace RegExp with string when string suffices.
* Remove conditions that cannot be false.
* Replace for..in with `Object.keys().forEach()`.

Also, eliminate needlessly complicated function chains:
* `ondone` callback only checks errors;
* if there is an error, it is called once and throws, then script exits;
* if there are no errors, it is noop;
* so there is no need to wrap it into `once()` function
* and there is no need to call it without errors;
* we can eliminate it and replace with `throw` where an error occurs;
* we can also replace `onprogress` callback with `console.log` in place;
* at last, we can eliminate `waiting` counter and `once()` utility.

The new script produces results identical to the old ones.

PR-URL: #20188
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 8, 2018
MylesBorins pushed a commit that referenced this pull request Aug 17, 2018
Modernize:
* Replace `var` with `const` / `let`.
* Replace common functions with arrow functions.
* Use destructuring.
* Use `String.prototype.padStart()`, `String.prototype.endsWith()`.

Optimize:
* Reduce function calls.
* Reduce intermediate variables.
* Cache retrieved object properties.
* Move RegExp declaration out of a cycle.
* Simplify RegExps.
* Replace RegExp with string when string suffices.
* Remove conditions that cannot be false.
* Replace for..in with `Object.keys().forEach()`.

Also, eliminate needlessly complicated function chains:
* `ondone` callback only checks errors;
* if there is an error, it is called once and throws, then script exits;
* if there are no errors, it is noop;
* so there is no need to wrap it into `once()` function
* and there is no need to call it without errors;
* we can eliminate it and replace with `throw` where an error occurs;
* we can also replace `onprogress` callback with `console.log` in place;
* at last, we can eliminate `waiting` counter and `once()` utility.

The new script produces results identical to the old ones.

PR-URL: #20188
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 17, 2018
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.

4 participants