PowerDNS
Require Developer Certificate of Origin in pull requests
Short description
This is only a draft for now, while we discuss how we want to do it.
Checklist
I have:
- [x] read the CONTRIBUTING.md document
- [ ] compiled this code
- [x] tested this code
- [ ] included documentation (including possible behaviour changes)
- [ ] documented the code
- [ ] added or modified regression test(s)
- [ ] added or modified unit test(s)
If we add DCO to the checklist, we can link to a relevant article there, in clear view of the submitter
If we add DCO to the checklist, we can link to a relevant article there, in clear view of the submitter
done
Fwiw, when github makes a squash commit, it lists github as the committer, and it does not include a sign-off-by: GitHub <[email protected]> in the commit message.
It blames the author as the PR author.
There's also this: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-the-commit-signoff-policy-for-your-repository#enabling-or-disabling-compulsory-commit-signoffs-for-your-repository
Looks good, I just enabled it. Thanks!
Pull Request Test Coverage Report for Build 17735527283
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 13629 unchanged lines in 213 files lost coverage.
- Overall coverage increased (+8.2%) to 65.996%
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| ext/lmdb-safe/lmdb-typed.cc | 1 | 80.0% |
| pdns/auth-catalogzone.hh | 1 | 66.67% |
| pdns/auth-zonecache.hh | 1 | 93.33% |
| pdns/backends/gsql/gsqlbackend.hh | 1 | 98.88% |
| pdns/dnsdistdist/dnsdist-lua-network.cc | 1 | 91.94% |
| pdns/dnswriter.hh | 1 | 76.6% |
| pdns/dynlistener.hh | 1 | 0.0% |
| pdns/inflighter.cc | 1 | 90.48% |
| pdns/recursordist/validate-recursor.cc | 1 | 64.94% |
| pdns/serialtweaker.cc | 1 | 84.15% |
| <!-- | Total: | 13629 |
| Totals | |
|---|---|
| Change from base Build 17658370954: | 8.2% |
| Covered Lines: | 128588 |
| Relevant Lines: | 166160 |
💛 - Coveralls
We agreed earlier this week that we still want to implement this. Does anyone have any issue with the current implementation?
1. The DCO mentions "under the open source license indicated in the file". Are we sure _all_ files have a license text? I don't think e.g. the docs have them.
Perhaps we can mention GPLv2 explicitly then?
2. Why are we not using an off-the-shelf DCO action like [this one](https://github.com/KineticCafe/actions-dco)?
I looked at what the existing options do, and their code, and I don't really want to talk about it. I have seen things.
3. Should all commits from author with `@powerdns.com` addresses be exempt?
I don't have a strong opinion, but that's easy to do.
2. Why are we not using an off-the-shelf DCO action like [this one](https://github.com/KineticCafe/actions-dco)?
Scratch that, I apparently missed this one in my last round and I have to say it looks quite OK. So why not, I'm always happy to have fewer lines of code to maintain.
Perhaps we can mention GPLv2 explicitly then?
If you do, point to NOTICE, as its not 'plain GPLv2'?
Perhaps we can mention GPLv2 explicitly then?
If you do, point to NOTICE, as its not 'plain GPLv2'?
The DCO also mentions that the text of itself should not be changed, where would we put this text?
So I switched to the action suggested by Pieter, after testing that it works as intended. I also replaced the DCO by a custom version that points to the NOTICE file. My understanding is that it is OK to do so, but I'm not a lawyer.
Added the AI policy, and mentioned in the pull request template and the DCO itself.
I don't quite get what the specll-checker is complaining about: "[here]( matches a line_forbidden.patterns entry: (?i)(?:>|\[)(?:(?:click |)here|link|(?:read |)more)(?:</|\]\(). (forbidden-pattern)", I guess it doesn't like the "here" link name?
#### Do not use `(click) here` links
For more information, see:
* https://www.w3.org/QA/Tips/noClickHere
* https://webaim.org/techniques/hypertext/link_text
* https://granicus.com/blog/why-click-here-links-are-bad/
* https://heyoka.medium.com/dont-use-click-here-f32f445d1021