Skip to content

Check that a semver-major PR has at least a CITGM CI run #141

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

Closed
joyeecheung opened this issue Jan 17, 2018 · 6 comments
Closed

Check that a semver-major PR has at least a CITGM CI run #141

joyeecheung opened this issue Jan 17, 2018 · 6 comments
Assignees
Labels
enhancement Things that enhances functionality, provided by node-core-utils feature request New features for node-core-utils pr-checker Issues related to pr checker

Comments

@joyeecheung
Copy link
Member

Refs: nodejs/node#18210

On a side note, we should check that the latest CI runs should be reasonably fresh (at least not older than a week?)

@apapirovski
Copy link
Contributor

Could we include semver-minor in this? In general, more PRs should get CitGM runs than is currently the case. I've started running it for any change I make that touches internals in a significant way, even if it seems completely harmless. The CitGM job doesn't even take much longer to finish than the main CI.

@joyeecheung
Copy link
Member Author

@apapirovski Well now come to think of it, maybe we should just create a label "citgm-needed"? The rule would be "semvor-major || citgm-needed".

I thought the reason why we don't run CITGM that often was because it's slow or we don't have enough resources (e.g. machines), but that might have changed or was never true... cc @nodejs/build @nodejs/citgm

@apapirovski
Copy link
Contributor

FWIW the last successful CitGM job took 1hr16min within most results being available after the first 30-40 minutes.

@joyeecheung joyeecheung added the feature request New features for node-core-utils label Jan 17, 2018
@joyeecheung joyeecheung self-assigned this Jan 17, 2018
@richardlau
Copy link
Member

Also there are modules tested by CitGM that are known to fail in master: nodejs/citgm#517

@gibfahn
Copy link
Member

gibfahn commented Jan 17, 2018

I thought the reason why we don't run CITGM that often was because it's slow or we don't have enough resources (e.g. machines), but that might have changed or was never true.

The biggest issue with CitGM is that it needs triage, modules fail all the time, usually unrelated to node core. There are also lots of modules with flaky tests. I don't think the runs have ever taken much more than an hour.

On a side note, we should check that the latest CI runs should be reasonably fresh (at least not older than a week?)

A week seems reasonable to me, after that the runs get removed from Jenkins anyway, so aren't really trustworthy.

@priyank-p priyank-p added the enhancement Things that enhances functionality, provided by node-core-utils label Jan 18, 2018
@joyeecheung joyeecheung added the pr-checker Issues related to pr checker label Jan 18, 2018
@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Things that enhances functionality, provided by node-core-utils feature request New features for node-core-utils pr-checker Issues related to pr checker
Projects
None yet
Development

No branches or pull requests

7 participants