Repository navigation
[CI][Doc] document GitHub e-mail privacy setting, post workflow comments on PRs - #23218
KornevNikita wants to merge 5 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.
|
@bader |
@KornevNikita, what is the reason to copy/paste from llvm-project? I expect us to re-use LLVM workflow rather than copy-paste. I don't see any reason why we shouldn't modify LLVM files (even if they are copy-pasted). Preferably modifications must be done in llvm-project files and propagated to intel/llvm with pulldown. |
Address review findings in the workflows ported from llvm/llvm-project. All four issues are present upstream as well and will be submitted there too, so that the next merge from upstream stays conflict-free. issue-write.yml: 1. The associated-pull-request loop used `=` instead of `===`, so the base-repository-owner check was assigned rather than compared and the condition reduced to `pr.number > pr_number`. A head branch that has pull requests against more than one repository could therefore select a number belonging to a different repository. This is easy to hit in this repository, where contributors routinely open pull requests from the same fork branch to both intel/llvm and llvm/llvm-project. 2. `updateComment()` is keyed only by `comment_id`; its `issue_number` argument does not constrain the update. Since the comment ids come from an artifact written by the unprivileged pull request workflow, verify that the comment being updated belongs to the pull request we are commenting on, not just that it was created by github-actions. unprivileged-download-artifact/action.yml: 3. The `filename` output referenced `steps.download-artifact.outputs.filename`, which that step never sets, so it was always empty. Replace it with `artifact-names`, fed by the `names` output that is already set, which also reflects that the action can download more than one artifact. 4. Fix the `ommitted` typo and the misspelled `desciption` input key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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