[JsConf Code & Learn]test: replace string concatenation with template literals#14322
[JsConf Code & Learn]test: replace string concatenation with template literals#14322nathansmile wants to merge 0 commit intonodejs:masterfrom
Conversation
test/parallel/test-icu-data-dir.js
Outdated
There was a problem hiding this comment.
ok, thks for suggestion.
Trott
left a comment
There was a problem hiding this comment.
Thanks for submitting this PR! The change in line 10 is not needed. The change in line 11 looks good to me, although I left a comment suggesting something that might be a bit better still. Can you update the PR at least to remove the changes in line 10?
test/parallel/test-icu-data-dir.js
Outdated
There was a problem hiding this comment.
This might be slightly better as:
`--icu-data-dir parameters)${os.EOL}`;|
@nathansmile Did you intentionally close this PR? It seems like you force-pushed the upstream master branch instead of your changes. |
|
When I'm updating the code, I don't want to add a new commit, or there will be 2 commits for this issue (I think it's not necessary). So I hard reset and force push the branch to my origin branch. After that I found this PR closed by me. What a surprise! Then I submit another PR. |
|
@nathansmile If you want to just update the last commit in a branch, you can try: git checkout your-branch
(do the change)
git commit --amend
(change the commit message if needed)
git push --force-with-leaseThe last commit will be updated in the PR automatically. |
FWIW I think once you push a new commit onto the branch you can reopen the PR. The easier fix is not to push between resetting and making the new commit. |
|
@vsemozhetbyt Thanks a lot~ That is what I need. |
replace string concatenation in test/parallel/test-icu-data-dir.js with template literals
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)