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.

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

PSA: it's now possible to start Jenkins CI by adding request-ci label to PRs #34594

Closed
mmarchini opened this issue Aug 1, 2020 · 14 comments
Closed
Labels
meta Issues and PRs related to the general management of the project.

Comments

@mmarchini
Copy link
Contributor

As title says, it's now possible to start Jenkins CI by adding request-ci label to PRs. CI will start a few minutes after the label is added. No extra involvement is needed from collaborators to start the CI after the label is added. Collaborators can add the label while creating new PRs, which will result in the CI starting soon after the PR is opened.

Since this was added recently, let's treat it as experimental for now (at least until it has been more extensively tested in this repository), but it should work on most cases. If anyone finds a bug with it, please leave a comment here. Once we have confidence this feature is stable we should update the collaborators guide to instruct new collaborators to use it as well.

@mmarchini mmarchini added the meta Issues and PRs related to the general management of the project. label Aug 1, 2020
@gengjiawen
Copy link
Member

I tried in #34629, looks like failed.

Not sure it's related to graphql query
https://github.com/nodejs/node/actions/runs/195904308

.github#L1
Unexpected input(s) 'owner', 'repo', valid inputs are ['query']

@targos
Copy link
Member

targos commented Aug 5, 2020

BTW it looks like the job's name is wrong (commitQueue)

@mmarchini
Copy link
Contributor Author

@gengjiawen that warning is ignorable, unfortunately I don't know how to supress it. Here's where the action failed: https://github.com/nodejs/node/runs/948079537?check_suite_focus=true. No relevant error messages. I'll increase verbosity of the logs to see if we get any clues.

@targos good catch, I'll fix the job name.

mmarchini added a commit to mmarchini/node that referenced this issue Aug 5, 2020
mmarchini added a commit that referenced this issue Aug 5, 2020
Ref: #34594 (comment)

PR-URL: #34635
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@mmarchini
Copy link
Contributor Author

@gengjiawen I tried again on the same PR and it worked 🤔. Let me know if you try again on another PR and it doesn't start the CI.

codebytere pushed a commit that referenced this issue Aug 6, 2020
Ref: #34594 (comment)

PR-URL: #34635
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
codebytere pushed a commit that referenced this issue Aug 11, 2020
Ref: #34594 (comment)

PR-URL: #34635
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@targos
Copy link
Member

targos commented Aug 18, 2020

In #34796, the second time I added request-ci, CI was started correctly but the bot didn't comment on the PR with the jenkins link.

@mmarchini
Copy link
Contributor Author

That's a Jenkins wide issue unfortunately (not related to the Action as far as we know): nodejs/build#2413

@ronag
Copy link
Member

ronag commented Aug 23, 2020

This is very cool. Though I just wanted to make sure we are aware that this basically bypasses the "certify safe" checkbox.

Screen Shot 2020-08-23 at 12 42 01

@lundibundi
Copy link
Member

@ronag yeah, there was a short discussion in #34089 about it but I guess we decided that it will be implicitly assumed when adding a label (though there is a short lag/time where someone can push changes before the CI starts which may be an issue). I guess we could put that in label description but I don't think it's noticeable there.

addaleax pushed a commit that referenced this issue Sep 22, 2020
Ref: #34594 (comment)

PR-URL: #34635
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Ref: #34594 (comment)

PR-URL: #34635
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@gengjiawen
Copy link
Member

Looks this one not triggered #35324
image

@mmarchini
Copy link
Contributor Author

Jenkins is shutting down, probably related

@gengjiawen
Copy link
Member

I am thinking if we can run extra CI job: https://ci.nodejs.org/job/node-test-commit-v8-linux/ if PR labeled with V8 Engine. cc @nodejs/v8

@mmarchini
Copy link
Contributor Author

Shouldn't be hard from an implementation perspective, we just need to align if that's the behavior we want

@targos
Copy link
Member

targos commented Jun 30, 2021

SGTM.

@mmarchini
Copy link
Contributor Author

Closing since this was a PSA and not a tracking issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

5 participants