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.

Sign up for GitHub

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

Jump to bottom

tools: remove unnecessary function declaration and make code a bit more readable #17125

Closed
wants to merge 2 commits into from
Closed

tools: remove unnecessary function declaration and make code a bit more readable #17125

wants to merge 2 commits into from

Conversation

tniessen
Copy link
Member

The first commit removes an unnecessary function declaration. The second commit makes some parts of the code a bit more readable in my opinion, but feel free to leave a comment / request changes if you would prefer not to change this.

Checklist
Affected core subsystem(s)

tools

@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 Nov 18, 2017
tniessen
tniessen commented Nov 18, 2017
View reviewed changes
tools/doc/generate.js
@@ -30,13 +30,13 @@ const fs = require('fs');
const args = process.argv.slice(2);
let format = 'json';
let template = null;
let inputFile = null;
let filename = null;
Copy link
Member Author

Choose a reason for hiding this comment

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

Using filename here is consistent with the name of the variable in other files and allows to pass the options object without renaming properties.

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 19, 2017
@addaleax
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-commit/14151/

@tniessen
Copy link
Member Author

@addaleax Thanks for kicking off CI, I didn't know this also covers the doc tool.

@tniessen
Copy link
Member Author

Landed in e515a0e, 2ba93f6.

tniessen added a commit that referenced this pull request Nov 21, 2017
@tniessen
tools: remove useless function declaration
e515a0e 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
tniessen added a commit that referenced this pull request Nov 21, 2017
@tniessen
tools: make doc tool a bit more readable
2ba93f6 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@tniessen tniessen closed this Nov 21, 2017
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@tniessen @MylesBorins
tools: remove useless function declaration
c8247a7 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@tniessen @MylesBorins
tools: make doc tool a bit more readable
5247ab3 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@tniessen @gibfahn
tools: remove useless function declaration
13b97e7 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@tniessen @gibfahn
tools: make doc tool a bit more readable
c5cd4f0 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@tniessen @gibfahn
tools: remove useless function declaration
9a76a6c 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@tniessen @gibfahn
tools: make doc tool a bit more readable
baf95b6 
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
@MylesBorins
Copy link
Contributor

@tniessen would you be able to backport to v6.x?

tniessen added a commit to tniessen/node that referenced this pull request Dec 20, 2017
@tniessen
tools: remove useless function declaration
5ef8f6d 
PR-URL: nodejs#17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
tniessen added a commit to tniessen/node that referenced this pull request Dec 20, 2017
@tniessen
tools: make doc tool a bit more readable
2b36cd5 
PR-URL: nodejs#17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@tniessen tniessen mentioned this pull request Dec 20, 2017
Closed
2 tasks
@tniessen
Copy link
Member Author

@MylesBorins #17788

MylesBorins pushed a commit that referenced this pull request Dec 20, 2017
@tniessen @MylesBorins
tools: remove useless function declaration
2526801 
Backport-PR-URL: #17788
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2017
@tniessen @MylesBorins
tools: make doc tool a bit more readable
d2f5084 
Backport-PR-URL: #17788
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 2, 2018
@tniessen @MylesBorins
tools: remove useless function declaration
b8a5d6d 
Backport-PR-URL: #17788
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 2, 2018
@tniessen @MylesBorins
tools: make doc tool a bit more readable
1d70602 
Backport-PR-URL: #17788
PR-URL: #17125
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@apapirovski apapirovski apapirovski approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@tniessen @addaleax @MylesBorins @apapirovski @jasnell @lpinca @cjihrig @gibfahn @nodejs-github-bot