chore: add dco check#271
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the pull request template by adding a checklist item for DCO sign-off. Feedback was provided to improve the terminology used for DCO signatures to avoid confusion with GPG signing and to correct the relative link path to the contributing documentation.
| - [ ] Format your code `make fmt`. | ||
| - [ ] Add unit tests or integration tests. | ||
| - [ ] Update the documentation related to the change. | ||
| - [ ] All commits are signed with DCO (`git commit -s`). See [Developer Certificate of Origin (DCO)](CONTRIBUTING.md#developer-certificate-of-origin-dco). |
There was a problem hiding this comment.
There are two minor improvements for this new checklist item:
- Terminology: Use "signed-off" instead of "signed". DCO (Developer Certificate of Origin) specifically uses the
Signed-off-bytrailer (added viagit commit -s), whereas "signed" often implies GPG/SSH signatures (git commit -S). This aligns with the terminology used inCONTRIBUTING.md(e.g., line 104). - Link Path: Since this template is located in the
.github/subdirectory, the relative link toCONTRIBUTING.mdshould be../CONTRIBUTING.mdto ensure it works correctly when viewing the template file directly in the repository browser.
| - [ ] All commits are signed with DCO (`git commit -s`). See [Developer Certificate of Origin (DCO)](CONTRIBUTING.md#developer-certificate-of-origin-dco). | |
| - [ ] All commits are signed-off with DCO (`git commit -s`). See [Developer Certificate of Origin (DCO)](../CONTRIBUTING.md#developer-certificate-of-origin-dco). |
8d2423c to
4d3343d
Compare
cheyang
left a comment
There was a problem hiding this comment.
Thanks for adding the DCO check — this is a good addition for the project. A couple of things to address before this can be merged:
1. The PR's own commit isn't signed off
The DCO Check is (correctly) failing on this PR because commit 4d3343d doesn't include a Signed-off-by line. You'll need to amend the commit:
git commit --amend -s
git push --force-with-lease2. Pin actions to a specific commit SHA
Using @master for third-party actions is a supply-chain security risk — upstream changes could silently alter behavior. Please pin both actions to a specific commit SHA (or at minimum a release tag):
- uses: tim-actions/get-pr-commits@<commit-sha>
- uses: tim-actions/dco@<commit-sha>You can find the latest release SHAs on the respective repos.
3. Minor — E2E test failure
The E2E test also failed in this run. It's likely unrelated since this PR only touches workflow config and the PR template, but worth confirming it's not a persistent issue on this branch.
Everything else looks good:
permissions: read-allis the right approach (least privilege).- The PR template reminder is a nice touch for contributor awareness.
- Trigger configuration (
pull_requestonmain) is correct.
Please fix items 1 and 2, and this should be ready to go.
Ⅰ. Motivation
Ⅱ. Modifications
Ⅲ. Does this pull request fix one issue?
fixes #XXXX
Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅴ. Describe how to verify it
VI. Special notes for reviews
Checklist
make fmt.