`needs-ci` label is confusing

[aduh95]

This may start to be getting off topic, but I suspect we also need to explain the needs-ci label (or get rid of it or rename it). Specifically, I'm under the impression it should remain after the CI is run and green because it's not a label that means "this needs a Jenkins run in the future" but rather "this needs a Jenkins run for it to land because it modifies executable code". I think a lot of people think it means that first thing and remove it after Jenkins starts or after it's green.

It may be too late to change this, but I wonder if in general, labels that are primarily for the bot/Actions should be prefixed with something so that it's clearer.

Originally posted by @Trott in #1039 (comment)

I'm fairly certain that label was repurposed and it was originally meant to be the latter before the contribution process changed to only requiring GitHub action runs for doc-only PRs.

Originally posted by @richardlau in #1039 (comment)

---

I agree, we should either rename it to requires-Jenkins-CI or similar, or have some kind of prefixing. Opening a new issue as I think it deserves its own discussion

[mmarchini]

How useful is the label still? ncu can tell if Jenkins CI is needed or not now.

[aduh95]

The idea was motivated by two things:

• help collaborators tell if a PR requires CI to land (without to have to run ncu).
• give the ability to collaborators to override ncu judgement (e.g., if a PR only changes a comment in a file, they could remove the label to notify that the PR doesn't actually need a full CI run).

[ljharb]

Couldn't that be achieved by a github status, which auto-succeeds if CI isn't required, and reports the CI status when it is required? (i guess that wouldn't cover the ability to override)

[legendecas]

and reports the CI status when it is required?

then it needs to re-run and update the status once Jenkins CI has been run for the PR?

[ljharb]

The CI process itself can update the status on completion.

[mhdawson]

@aduh95 is this issue still useful? Only asking because it's been a few years since the last discussion.

[GeoffreyBooth]

Someone just triggered a fresh CI job on a PR of mine that had a passing CI, that I was waiting for the time limit to expire in order to land. Now we’re going to go into another cycle of “resume job” runs for the next several hours, because a collaborator misinterpreted “needs CI” to mean, well, that the PR needs CI to be run for it. We need to rename this terrible label ASAP.

Perhaps ci-must-pass-to-land; and it would be nice if a bot automatically added ci-passed after a passing CI, and removed it if additional commits get pushed.

[joyeecheung]

I think we can just remove it. The CI requirement is already shown at the button of the GitHub UI.

[tniessen]

The CI requirement is already shown at the button of the GitHub UI.

@joyeecheung Could you clarify what UI element you refer to? I must have never noticed that.

[GeoffreyBooth]

@joyeecheung Could you clarify what UI element you refer to? I must have never noticed that.

Perhaps Joyee means the box above the merge button, that lists all the results of the GitHub Actions jobs? As in, if Actions jobs run, then by definition this PR needs CI.

[joyeecheung]

Yes, although I now realize that there would just be GitHub actions. We could also just..switch it to request-ci. Probably going to do more good than harm.

[joyeecheung]

Oh wait, someone can still run arbitrary code via a PR so it should not be request-ci automatically. Maybe we can instead modify the bot to say something smarter instead, it already comments to ping the reviewers anyway...we can even make it smarter, if the test only touches documentation, it can say this only requires doc + linter tests. If the test touches anything else it can say this requires full test. If the test touches deps/v8 it can say this requires v8-commit tests.

[joyeecheung]

Oh, if we can install this https://plugins.jenkins.io/build-with-parameters/ we can even let the bot generate a URL that fills out the build parameters automatically

[joyeecheung]

I just noticed that needs-ci is used by git node land to recognize PRs that needs full CI runs: nodejs/node#37308 (comment)