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: rename unused variale in more pythonic way #16171

Closed
wants to merge 1 commit into from

Conversation

komawar
Copy link
Contributor

@komawar komawar commented Oct 12, 2017

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

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
Member

@joyeecheung
Copy link
Member

@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
Member

@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
Member

joyeecheung commented Oct 16, 2017

@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
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
Member

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
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
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
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
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
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
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
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
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
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
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants