Skip to content

src: don't store one-use strings in variable #21876

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
Closed
wants to merge 1 commit into from
Closed

src: don't store one-use strings in variable #21876

wants to merge 1 commit into from

Conversation

@maclover7
Copy link
Contributor

@maclover7 maclover7 commented Jul 19, 2018

Move strings that are used only once to their call-sites, don't store
in a variable. What most other places in src/cares_wrap.cc do, as well as other files in src.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 19, 2018

@maclover7 build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/311/pipeline
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jul 19, 2018
@addaleax
Copy link
Member

addaleax commented Jul 19, 2018

@maclover7 I’d also label this with src as the subsystem, btw. The main purpose of adding those to commit messages is to make changes to certain subsystems easier to spot in changelogs, but this does not actually change the behaviour of the dns module :)
@maclover7
src: don't store one-use strings in variable
801ee85 
Move strings that are used only once to their call-sites, don't store in a variable.
@maclover7 maclover7 changed the title dns: don't store one-use strings in variable src: don't store one-use strings in variable Jul 19, 2018
@maclover7
Copy link
Contributor Author

maclover7 commented Jul 19, 2018

Full CI: https://ci.nodejs.org/job/node-test-pull-request/15949/
@maclover7 maclover7 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 20, 2018
@maclover7
Copy link
Contributor Author

maclover7 commented Jul 22, 2018

Landed in de8a8d0
@maclover7 maclover7 closed this Jul 22, 2018
@maclover7 maclover7 deleted the jm-mv-str branch July 22, 2018 13:49
maclover7 added a commit that referenced this pull request Jul 22, 2018
@maclover7
src: don't store one-use strings in variable
de8a8d0 
Move strings that are used only once to their call-sites, don't store in a variable. PR-URL: #21876 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@maclover7 maclover7 restored the jm-mv-str branch July 22, 2018 15:42
@maclover7 maclover7 reopened this Jul 22, 2018
@maclover7 maclover7 closed this Jul 22, 2018
@maclover7 maclover7 deleted the jm-mv-str branch July 22, 2018 15:42
targos pushed a commit that referenced this pull request Jul 24, 2018
@maclover7 @targos
src: don't store one-use strings in variable
be75795 
Move strings that are used only once to their call-sites, don't store in a variable. PR-URL: #21876 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos targos mentioned this pull request Jul 31, 2018
Merged
This was referenced Aug 1, 2018
Closed
Closed
Closed
Closed
Closed
@beevelop beevelop mentioned this pull request Aug 2, 2018
Closed
This was referenced Jun 18, 2021
Open
Open
Open
Open
Open
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. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.

9 participants

@maclover7 @nodejs-github-bot @addaleax @jasnell @TimothyGu @cjihrig @JungMinu @devsnek @trivikr