GeezerVison

@JakobJingleheimer I saw that you have been performing a codereview operation recently. Can you help me check it ~ 😉

Note that your commit summary does not follow the required pattern. I think it should be http: remove useless ternary in test (or perhaps test: remove…). I'm not clear on the use of test: now that it's also a core module (previously it was used for changes solely affecting nodes own tests).

Note that your commit summary does not follow the required pattern. I think it should be http: remove useless ternary in test (or perhaps test: remove…). I'm not clear on the use of test: now that it's also a core module (previously it was used for changes solely affecting nodes own tests).

I have corrected it, thank you for your guidance ❤️

It looks like you changed the PR title but not the commit message. The PR summary can be whatever you want, but the first commit is what CI checks (and prevents landing when invalid).

You'll need to run

git commit --amend -m "http: remove useless ternary in test"
git push --force

PS your change to the PR summary is missing a space character between the colon and the next word (test:remove …), which would fail the pattern check (so don't copy+paste your PR title—you don't need to correct the PR title, but you can if you want).

PPS A forced push will invalidate my approval; I'll re-approve after you push ;) I think it will send me a notification; if you do it in the next ~½ hour, I'm around. If I haven't re-approved in like 20 mins, feel free to re-request a review from me.

It looks like you changed the PR title but not the commit message. The PR summary can be whatever you want, but the first commit is what CI checks (and prevents landing when invalid).

You'll need to run

git commit --amend -m "http: remove useless ternary in test"
git push --force

PS your change to the PR summary is missing a space character between the colon and the next word (test:remove …), which would fail the pattern check (so don't copy+paste your PR title—you don't need to correct the PR title, but you can if you want).

Thank you very much. I have learned a lot, and my foundation still needs to be strengthened 😢

Everyone starts somewhere 🙌

And thanks for the contribution!

@mscdex Here~~，Can you give me an approval 😉

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

Rebasing shouldn't be necessary. If you start a new CI run, Jenkins will rebase by default:

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

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

I performed a rebase operation ~ 😉❤️

I think you missed Michaël's comment that it's actually unnecessary (my mistake—I'm usually supplying the PRs, not landing them 😅). I restarted the landing process though 🙂

Commit Queue failed
🤔🤔,I have read some error logs and it doesn't feel like they were caused by changes 😕

again 😢 ... error msg like【tap2junit: error: argument --input/-i: can't open 'cctest.tap': [Errno 2] No such file or directory: 'cctest.tap'】,Should this be a problem with ci itself ？Can we skip them😢

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

Commit Queue failed
🤔🤔,I have read some error logs and it doesn't feel like they were caused by changes 😕

again 😢 ... error msg like【tap2junit: error: argument --input/-i: can't open 'cctest.tap': [Errno 2] No such file or directory: 'cctest.tap'】,Should this be a problem with ci itself ？Can we skip them😢

Did this same error occur multiple runs?

It's very common to have to run CI half a dozen or more times to get a passing suite (we track this, and it's gotten better, but the ones that remain have proven difficult to squash). I checked another PR also currently going through CI, and I did not see this error there. We can't selectively skip tests, however unrelated the failures are :(

I resumed the failing tests, so 🤞

Landed in 0d1dfe1