★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21938Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

doc: replace deprecated CI job #21938

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@joaocgreis joaocgreis

@jasnell jasnell

@lpinca lpinca

@cjihrig cjihrig

@maclover7 maclover7

@riyadhalnur riyadhalnur

@richardlau richardlau

@trivikr trivikr

Assignees
No one assigned
Labels
doc meta
Projects
None yet
Milestone
No milestone
10 participants
@vsemozhetbyt @nodejs-github-bot @joaocgreis @jasnell @lpinca @cjihrig @maclover7 @riyadhalnur @richardlau @trivikr
@vsemozhetbyt
Copy link
Member

vsemozhetbyt commented Jul 22, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

node-test-pull-request-lite CI job is deprecated now and https://ci.nodejs.org/job/node-test-pull-request-lite/ has this note: "DEPRECATED: please use the node-test-pull-request-lite-pipeline job".
@vsemozhetbyt
doc: replace deprecated CI job 
`node-test-pull-request-lite` CI job is deprecated now and
https://ci.nodejs.org/job/node-test-pull-request-lite/ has this note:
"DEPRECATED: please use the node-test-pull-request-lite-pipeline job".
Loading status checks…
8182401
@nodejs-github-bot

@nodejs-github-bot nodejs-github-bot added the doc label Jul 22, 2018

@vsemozhetbyt vsemozhetbyt added the meta label Jul 22, 2018

@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsemozhetbyt commented Jul 22, 2018

Not sure if this should be approved by the TSC or any other WG. Please, cc or label as seems appropriate.
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsemozhetbyt commented Jul 22, 2018

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.
@richardlau

richardlau approved these changes Jul 22, 2018
View changes

Copy link
Member

richardlau left a comment

Comment is non-blocking for me.
doc/onboarding.md
@@ -213,7 +213,7 @@ needs to be pointed out separately during the onboarding.
* Optionally, include your personal pronouns.
* Label your pull request with the `doc` and `notable-change` labels.
* Run CI on the PR. Because the PR does not affect any code, use the
`node-test-pull-request-lite` CI task. Alternatively, use the usual
`node-test-pull-request-lite-pipeline` CI task. Alternatively, use the usual
`node-test-pull-request` CI task and cancel it after the linter and one other
subtask have passed.

This comment has been minimized.

Sign in to view
Copy link
@richardlau

richardlau Jul 22, 2018

Member

Maybe one for @nodejs/build to comment on, but my own experience with Jenkins is that cancelling jobs can sometimes leave workspaces in a bad state (usually manifesting in git checkout errors on subsequent runs) depending on where the job was at at the time it was cancelled so I'm not generally in favour of encouraging the cancellation of jobs.

This comment has been minimized.

Sign in to view
Copy link
@maclover7

maclover7 Jul 23, 2018

Member

From my personal experience, a lot of git checkout errors / infra failures in general with ci.nodejs.org can be linked back to issues with the machine itself, not usually due to cancelling jobs early. Also, you could say there's a benefit from new collaborators practicing how to launch node-test-pull-request jobs. I'd rather folks let the build go all the way through and finish, but I don't think they're "breaking our infra" by cancelling it early.

This comment has been minimized.

Sign in to view
Copy link
@joaocgreis

joaocgreis Jul 23, 2018

Member

I'd prefer that cancelling jobs is not encouraged at all, I've seen a lot of failures because of it. It may not be the most frequent issue but it happens.

At least two cases are know: 1) Abort during a git operation will leave behind a lock file that will make all following jobs on that machine fail; 2) Visual Studio will not stop running immediately after the job is aborted, locking some files and making git clean fail for some time/jobs (happens easily when there are queued jobs). Note that Jenkins does not offer us any reliable way of running anything between jobs, so fixing this is not straightforward.

(To be clear, I'm also not blocking this PR, but would be in favor of deleting the sentence starting at "Alternatively".)

This comment has been minimized.

Sign in to view
Copy link
@vsemozhetbyt

vsemozhetbyt Jul 24, 2018

Author Member

I will make a new PR after landing to draw more attention to this change.

This comment has been minimized.

Sign in to view
Copy link
@vsemozhetbyt

vsemozhetbyt Jul 25, 2018

Author Member

PR: #21977
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsemozhetbyt commented Jul 22, 2018

cc @nodejs/automation It seems the git-node needs updating its lite CI detection.
@cjihrig

cjihrig approved these changes Jul 22, 2018
View changes

@riyadhalnur

riyadhalnur approved these changes Jul 23, 2018
View changes

@trivikr

trivikr approved these changes Jul 23, 2018
View changes

@jasnell

jasnell approved these changes Jul 23, 2018
View changes

@maclover7

maclover7 approved these changes Jul 23, 2018
View changes

@lpinca

lpinca approved these changes Jul 23, 2018
View changes

@joaocgreis

joaocgreis approved these changes Jul 23, 2018
View changes

@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member Author

vsemozhetbyt commented Jul 25, 2018

Landed in c75f36f
Thank you for the reviews.

@vsemozhetbyt vsemozhetbyt closed this Jul 25, 2018

@vsemozhetbyt vsemozhetbyt deleted the vsemozhetbyt:doc-replace-deprecated-ci-job branch Jul 25, 2018

vsemozhetbyt added a commit that referenced this pull request Jul 25, 2018

@vsemozhetbyt
doc: replace deprecated CI job 
`node-test-pull-request-lite` CI job is deprecated now and
https://ci.nodejs.org/job/node-test-pull-request-lite/ has this note:
"DEPRECATED: please use the node-test-pull-request-lite-pipeline job".

PR-URL: #21938
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Loading status checks…
c75f36f

vsemozhetbyt added a commit to vsemozhetbyt/node that referenced this pull request Jul 25, 2018

@vsemozhetbyt
doc: do not advise to cancel full CI on onboarding 
Refs: nodejs#21938 (comment)
and discussion below the comment.
bbc85a8

@vsemozhetbyt vsemozhetbyt referenced this pull request Jul 25, 2018

Closed

doc: do not advise to cancel full CI on onboarding #21977

3 of 3 tasks complete

targos added a commit that referenced this pull request Jul 26, 2018

@vsemozhetbyt @targos
doc: replace deprecated CI job 
`node-test-pull-request-lite` CI job is deprecated now and
https://ci.nodejs.org/job/node-test-pull-request-lite/ has this note:
"DEPRECATED: please use the node-test-pull-request-lite-pipeline job".

PR-URL: #21938
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
478dbee

vsemozhetbyt added a commit that referenced this pull request Jul 26, 2018

@vsemozhetbyt
doc: do not advise to cancel full CI on onboarding 
Refs: #21938 (comment)
and discussion below the comment.

PR-URL: #21977
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Loading status checks…
a9f32a3

targos added a commit that referenced this pull request Jul 26, 2018

@vsemozhetbyt @targos
doc: do not advise to cancel full CI on onboarding 
Refs: #21938 (comment)
and discussion below the comment.

PR-URL: #21977
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
Loading status checks…
2a0fa47

@vsemozhetbyt vsemozhetbyt referenced this pull request Jul 31, 2018

Closed

git-node: count jenkins pipeline and/or Travis as effective CI runs #259

@targos targos referenced this pull request Jul 31, 2018

Merged

v10.8.0 proposal #22040

This was referenced Aug 1, 2018

Closed
Closed
Closed
Closed

This was referenced Aug 2, 2018

Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.