Ping TSC on deps update not from GithubBot #1329
Comments
|
I strongly agree |
|
To create a comment in a PR in nodejs/node repository, even when providing a PR from a fork you need to listen on a I dont think we need to write a new action. So something like the following could be enough? Untested(!) on:
pull_request_target:
paths:
- 'deps/**'
jobs:
ping-tsc:
runs-on: ubuntu-latest
name: Ping the TSC
steps:
- name: Ping the TSC
if: >
github.event.pull_request.user.login != 'nodejs-github-bot'
uses: thollander/actions-comment-pull-request@v2
with:
message: Pinging @tsc because of an irregular change in the deps folder
comment_tag: ping-tsc |
|
Maybe it makes also sense to add a "blocked" label to the PR. |
Pinging the TSC for changes to deps should be easy (add them as a CODEOWNER and then our bot should then ping the team in a comment to the PR) but we have no logic AFAIK for checking who originated the PR. |
|
I can make a workflow if you'd like |
|
|
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]>
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]>
|
PR - to add guidance to contributing docs - nodejs/node#53499 |
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: #53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
|
Thinking about this a bit, instead of a ping to the TSC, I think adding a comment to the PR with a warning to collaborators that updates to dependencies should generally be generated by the automation and pointing to https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#updating-dependencies for more information would be more beneficial. |
|
I can modify my PR if you'd like |
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: #53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: #53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
- add additional guidance based in discussion related to recent PR to dependency and discussion within the security-wg slack channel. Refs: nodejs/security-wg#1329 Signed-off-by: Michael Dawson <[email protected]> PR-URL: #53499 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
|
This issue has been inactive for 90 days. It will be closed in 14 days unless there is further activity or the stale label is taken off. |
We receive dependency update such as nodejs/node#53406 that are created by users that might be hard and risky to review. We should probably ping TSC whenever we receive changes in the deps folder not from a GithubBot
The text was updated successfully, but these errors were encountered: