Skip to content

tools: rename unused variale in more pythonic way#16171

Closed
komawar wants to merge 1 commit intonodejs:masterfrom
komawar:unused-var-pythonic
Closed

tools: rename unused variale in more pythonic way#16171
komawar wants to merge 1 commit intonodejs:masterfrom
komawar:unused-var-pythonic

Conversation

@komawar
Copy link
Copy Markdown
Contributor

@komawar komawar commented Oct 12, 2017 •
edited
Loading

The 'Main' function in tools/test.py file was using a variable named
all_outcomes to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as _ [1]. This just helps keep the code a bit cleaner and
avoid any silly mistakes.

Refs: [1] https://stackoverflow.com/a/5477153

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

NA

@komawar
tools: rename unused variale in more pythonic way
b6ccc4a 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner and
avoid any silly mistakes.

Refs: [1] https://stackoverflow.com/a/5477153
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Oct 12, 2017
@gireeshpunathil
Copy link
Copy Markdown
Member

gireeshpunathil commented Oct 16, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/10736/

@joyeecheung
Copy link
Copy Markdown
Member

joyeecheung commented Oct 16, 2017

@gireeshpunathil The CI link doesn't seem to be correct? It links to another PR.

Another CI: https://ci.nodejs.org/job/node-test-pull-request/10740/

@gireeshpunathil
Copy link
Copy Markdown
Member

gireeshpunathil commented Oct 16, 2017

@joyeecheung - sorry, and thanks for correcting it. Looking back at what would have happened - I guess I clicked the build button and selected the latest build identifier from the left hand panel, which probably wasn't updated by then. thanks once again for catching this.

@joyeecheung
Copy link
Copy Markdown
Member

joyeecheung commented Oct 16, 2017 •
edited
Loading

@gireeshpunathil No worries, that's basically what I do as well, I would usually wait a while for the revision to show up to see if they match though.

@jasnell
Copy link
Copy Markdown
Member

jasnell commented Oct 18, 2017

Retrying CI on Arm: https://ci.nodejs.org/job/node-test-binary-arm/11029/

@BridgeAR BridgeAR self-assigned this Oct 18, 2017
@BridgeAR
Copy link
Copy Markdown
Member

BridgeAR commented Oct 19, 2017

Landed in 5a17ab3

Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 !

@BridgeAR BridgeAR closed this Oct 19, 2017
BridgeAR pushed a commit that referenced this pull request Oct 19, 2017
@komawar @BridgeAR
tools: rename unused variale in more pythonic way
5a17ab3 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: #16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@komawar
Copy link
Copy Markdown
Contributor Author

komawar commented Oct 19, 2017

Thanks all for the reviews! Look forward to jump in more into the community ;)

MylesBorins pushed a commit that referenced this pull request Oct 23, 2017
@komawar @MylesBorins
tools: rename unused variale in more pythonic way
118724f 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: #16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
@komawar @addaleax
tools: rename unused variale in more pythonic way
017b0d8 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: nodejs/node#16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@komawar @MylesBorins
tools: rename unused variale in more pythonic way
6e4fa6a 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: #16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@komawar @MylesBorins
tools: rename unused variale in more pythonic way
0433a60 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: #16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@komawar @MylesBorins
tools: rename unused variale in more pythonic way
b48471a 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: #16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
@komawar @addaleax
tools: rename unused variale in more pythonic way
28df95b 
The 'Main' function in tools/test.py file was using a variable named
``all_outcomes`` to store a value not being used. It is a best practice
to name unused variables, often return values of functions/methods (as
in this case) as ``_`` [1]. This just helps keep the code a bit cleaner
and avoid any silly mistakes.

PR-URL: nodejs/node#16171
Refs: [1] https://stackoverflow.com/a/5477153
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

+1 more reviewer

@claudiorodriguez claudiorodriguez claudiorodriguez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@BridgeAR BridgeAR

Labels

test Issues and PRs related to the tests. 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

@komawar @gireeshpunathil @joyeecheung @jasnell @BridgeAR @cjihrig @claudiorodriguez @MylesBorins @nodejs-github-bot