Skip to content

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

Closed
vsemozhetbyt wants to merge 1 commit intonodejs:v6.x-stagingfrom
vsemozhetbyt:backport-12735-to-v6.x
Closed

(v6.x backport) test: reduce string concatenations#13835
vsemozhetbyt wants to merge 1 commit intonodejs:v6.x-stagingfrom
vsemozhetbyt:backport-12735-to-v6.x

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

vsemozhetbyt commented Jun 21, 2017

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

vsemozhetbyt commented Jun 23, 2017

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 <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 23, 2017

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

MylesBorins commented Jul 10, 2017

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 <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@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 <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
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