[CI][Doc] document GitHub e-mail privacy setting, post workflow comments on PRs - #23218
KornevNikita wants to merge 4 commits into
Conversation
email-check.yaml writes its warning into a `comments` artifact, but nothing in this repo consumed that artifact, so the comment was never posted. Port issue-write.yml and the unprivileged-download-artifact action from llvm/llvm-project to post it, bring email-check.yaml up to its current upstream state, and document the setting in CONTRIBUTING.md and ContributeToDPCPP.md. pr-code-format.yml uploads its comments through the same artifact, so the clang-format comments, which were silently dropped for the same reason, should start being posted on PRs as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
deliberately canceled win & linux pre-commit as they're no affected. |
|
I think @uditagarwal97 worked on this before and if I remember correctly he determined it wasn't possible to accurately do the check with any GitHub API, maybe he can take a look here |
| ## Commit e-mail address | ||
|
|
||
| Please make sure that the e-mail address recorded in your commits is a real one | ||
| and not a GitHub `noreply` address: turn off the | ||
| [Keep my email addresses private](https://github.com/settings/emails) setting in | ||
| your GitHub account and check that your local `git config user.email` matches one | ||
| of the verified addresses of that account. Pull requests authored with a private | ||
| e-mail address are automatically flagged with a warning comment. See | ||
| [LLVM Developer Policy](https://llvm.org/docs/DeveloperPolicy.html#email-addresses) | ||
| and this [LLVM Discourse thread](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it) | ||
| for more information. |
There was a problem hiding this comment.
As per my understanding, the email address used to author a commit is different from the email address that Github will use when merging a PR. So, even if the author signed their commits but have email address private (in Github Settings), Github will still use noreply email address.
For example: in #23063, the email used to author commits is an intel email (https://github.com/intel/llvm/commit/a7d0ca457248480674293968f11b4a34ad614cb9.patch), but Github still choses to use noreply anonymous email for merging.
There was a problem hiding this comment.
There are Github APIs to get the email id with which commits are authored, but there is no API to get user's "Keep my email private" setting. So, the email-check workflow is not of much use, IMO.
There was a problem hiding this comment.
yeah, you're right, thanks for the clarification. Then I'll reduce this patch to adding a note for contibutors.
Anyways, I guess we should keep .github/workflows/issue-write.yml to issue a comment about format
GitHub records the author of a merged commit using the e-mail address from the account settings, not the one used in the pull request commits, and there is no API to query the "Keep my email addresses private" setting. The email-check workflow, which only inspects the commit author e-mail, therefore can't detect the problem, so remove it and document the setting in CONTRIBUTING.md only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KornevNikita
changed the title
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KornevNikita
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
PR selection is incorrect and attacker-controlled artifacts can overwrite unrelated bot comments.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
Documents GitHub email privacy requirements and adds privileged workflow support for posting CI artifact comments to pull requests.
Changes:
- Documents commit email configuration.
- Adds artifact download composite action.
- Adds workflow for creating or updating PR comments.
| File | Description |
|---|---|
CONTRIBUTING.md |
Documents email privacy requirements. |
.github/workflows/unprivileged-download-artifact/action.yml |
Downloads workflow artifacts. |
.github/workflows/issue-write.yml |
Posts artifact-generated comments to PRs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (old_comment.data.user.login != "github-actions[bot]") { | ||
| console.log("Invalid comment id: " + comment.id); | ||
| return; | ||
| } |
| // The largest PR number is the one we care about. The only way | ||
| // to have more than one associated pull requests is if all the | ||
| // old pull requests are in the closed state. | ||
| if (pr.baseRepository.owner.login = context.repo.owner && pr.number > pr_number) { |
| filename: | ||
| description: >- | ||
| The filename of the downloaded artifact or the empty string if the | ||
| artifact was not found. | ||
| value: ${{ steps.download-artifact.outputs.filename }} |
| run-id: | ||
| description: >- | ||
| The run-id for the workflow run that you want to download the artifact | ||
| from. If ommitted it will download the most recently created artifact |
Document the "Keep my email addresses private" GitHub setting in
CONTRIBUTING.md, which GitHub links to when a PR is created. When a PR ismerged, GitHub records the author of the resulting commit using the e-mail
address from the account settings, not the one used to author the PR
commits, so a real
git config user.emailalone is not enough.pr-code-format.yml writes its comments into a
commentsartifact, butnothing in this repository consumes that artifact, so the comments are never
shown on the PR. In
llvm/llvm-projectthe artifact is picked up by aprivileged
issue-write.ymlworkflow, which we don't have.Port
issue-write.ymland theunprivileged-download-artifactcompositeaction from
llvm/llvm-project. Both are copied verbatim (including theworkflows:list, so that future upstream merges stay conflict-free), theonly change is the repository condition, which now also allows
intel/llvm,the same way
pr-code-format.ymlalready does.Note that email-check.yaml uses the same artifact, so its warning about a
noreplycommit author e-mail will start appearing on PRs as well.🤖 Generated with Claude Code