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

(v6.x backport) test: reduce string concatenations #13835

Closed
wants to merge 1 commit into from
Closed

(v6.x backport) test: reduce string concatenations #13835

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Jun 21, 2017 •
edited
Loading

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

test

Backport of #12735

@nodejs-github-bot nodejs-github-bot added addons Issues and PRs related to native addons. debugger inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Jun 21, 2017
@vsemozhetbyt vsemozhetbyt mentioned this pull request Jun 21, 2017
Closed
24 tasks
@vsemozhetbyt vsemozhetbyt added refactor to ES6+ and removed addons Issues and PRs related to native addons. debugger inspector Issues and PRs related to the V8 inspector protocol labels Jun 21, 2017
@vsemozhetbyt
Copy link
Contributor Author

WRT assignment: #12735 (comment)

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 21, 2017 •
edited
Loading

Can a CI for a backport be launched as a common CI (with setting a PR ID in the same form)?

@gibfahn
Copy link
Member

gibfahn commented Jun 21, 2017 •
edited
Loading

Can a CI for a backport be launched as a common CI (with setting a PR ID in the same form)?

Yes. You just leave the Rebase field as the default (so it rebases on the PR base branch).

That should probably be documented.

@gibfahn gibfahn mentioned this pull request Jun 21, 2017
Merged
3 tasks
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 21, 2017 •
edited
Loading

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

(I hope I've set it right, as I usually do not change the Rebase field for common CIs as well)

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 21, 2017 •
edited
Loading

One arm fail on pre-test phase if I get it right.

@gibfahn
Copy link
Member

gibfahn commented Jun 21, 2017

@refack you up for a round 2 of reviewing this PR?

image

@refack
Copy link
Contributor

refack commented Jun 21, 2017

@refack you up for a round 2 of reviewing this PR?

I'll try to compare to #12735

@refack
Copy link
Contributor

refack commented Jun 21, 2017

http://science.refack.com.s3.amazonaws.com/12735-13835-diff.html

@vsemozhetbyt
Copy link
Contributor Author

How should I resolve conflicts in backports? git pull --rebase upstream v6.x-staging?

@gibfahn
Copy link
Member

gibfahn commented Jun 23, 2017

How should I resolve conflicts in backports? git pull --rebase upstream v6.x-staging?

Yep

@vsemozhetbyt
test: reduce string concatenations
e0d8afa 
PR-URL: #12735
Refs: #12455
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@vsemozhetbyt
Copy link
Contributor Author

Conflict resolved. New CI: https://ci.nodejs.org/job/node-test-pull-request/8810/

@refack refack mentioned this pull request Jul 4, 2017
Closed
4 tasks
@MylesBorins
Copy link
Contributor

Landed in 0dfb19d

My gut tells me this should be fine, but I am kinda concerned there may be perf regressions on V8 5.1

/cc @nodejs/v8 are there any issues with using string interpolation on V8 5.1?

MylesBorins pushed a commit that referenced this pull request Jul 10, 2017
@vsemozhetbyt @MylesBorins
test: reduce string concatenations
35b55bf 
Backport-PR-URL: #13835
PR-URL: #12735
Refs: #12455
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@vsemozhetbyt vsemozhetbyt deleted the backport-12735-to-v6.x branch July 10, 2017 13:51
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
@vsemozhetbyt @MylesBorins
test: reduce string concatenations
5b74e63 
Backport-PR-URL: #13835
PR-URL: #12735
Refs: #12455
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gibfahn gibfahn

Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vsemozhetbyt @gibfahn @refack @MylesBorins @nodejs-github-bot