node-core-utils icon indicating copy to clipboard operation
node-core-utils copied to clipboard

feat(ncu-ci): parse comments to find a safe commit

Open aduh95 opened this issue 10 months ago • 4 comments

Currently, the request-ci only works for approved PRs. This is annoying because:

  • collaborators cannot approve their own PRs, and have to wait for someone else to approve their PR for them, or start the CI manually.
  • it gives an incentive to approve a PR just to run the CI rather than because the collaborator actually supports it.

There have been suggestions to use PR author, or PR head repo owner, or commit signature, or a combination of the three – but I don't really like that idea, because:

  • it doesn't help for PRs from non-collaborators.
  • we should care about who is requesting the CI, not who's proposing the change.

IIUC comments are not editable by triagers, so should be a safe way to determine a safe SHA for unapproved PRs.

aduh95 avatar Apr 05 '25 16:04 aduh95

IIUC comments are not editable by triagers, so should be a safe way to determine a safe SHA for unapproved PRs.

yes, only people with write access have the ability to edit comments

bjohansebas avatar Apr 05 '25 17:04 bjohansebas

Would you like a review before updating the tests?

targos avatar Apr 06 '25 10:04 targos

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.82%. Comparing base (ef135f5) to head (733bd44). Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #920      +/-   ##
==========================================
+ Coverage   79.70%   79.82%   +0.12%     
==========================================
  Files          39       39              
  Lines        4642     4650       +8     
==========================================
+ Hits         3700     3712      +12     
+ Misses        942      938       -4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Apr 06 '25 14:04 codecov[bot]

AFAICT, this allows to request CI for a commit SHA that is not part of the pull request. Is that a bug or a feature?

The commit SHA provided by the collaborator with be passed as COMMIT_SHA_CHECK , Jenkins checks that GIT_REMOTE_REF (i.e. refs/pull/${prID}/head) resolves to that expected commit SHA, otherwise the CI fails. So if someone pass the SHA of a commit not in the PR branch, it won't match the tip of the PR head, and it won't work

aduh95 avatar Apr 09 '25 15:04 aduh95