[Openvpn-devel,v2] Start a new document CODE_CHECKLIST
Commit Message
From: Frank Lichtenheld <frank@lichtenheld.com>
This is intended to document in one place all the policies
applicable to submitted changes. This serves multiple
purposes:
* Make it clearer to submitters what is required of them
* Make it clearer what to consider automating when developing
(be it by writing scripts or by instructing an AI agent)
* Reduce the bus factor of the project by reducing the amount
of things only stored in Gert's head
Change-Id: Ib123b7bf58317a1be5af1a1d45decb8f31f2ec68
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1776
---
This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1776
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
new file mode 100644
@@ -0,0 +1,115 @@
+# Checklist for Contributions
+
+## Introduction
+
+This documents all policies applied to any change that should be applied
+in this repository. It is separated in two parts. The first part describes
+the parts of the checklist that any submitter should check. The seconds part
+describes the parts that integrators need to check before actually merging the
+change.
+
+In general the Integrator Checklist does imply that all checks from the
+Submitter Checklist have been passed. They are not repeated.
+
+## Submitter Checklist
+
+### Commit message
+
+* Does the commit message acurately describe the change?
+* Does it end with the line `Signed-off-by: My Name <my@email.com>` ?
+
+### Automated tests
+
+* Did the change pass a local `make` build?
+* Did the change pass a local run of `make check`?
+* Have you verified the code formatting with clang-format?
+* Is the code covered by Unit Tests? If not, have you considered adding
+ a new Unit Test?
+
+### Change-specific checks
+
+* Does your change require adaptions on both server and client side
+ because it changes the wire-protocol?
+ * Did you propose a change to http://github.com/openvpn-rfc/
+ to agree on the protocol format changes?
+* Does your change add, remove, or change a config option?
+ * Have you adapted the usage text (`openvpn --help`)?
+ * Have you adapted the man page text (`man -l ./doc/openvpn.8`)?
+* Have you reviewed all code comments around code you touched to
+ check whether they need to be adapted?
+* If your change adds new files, are they added in some Makefile.am
+ statement that will make sure they up in the distributed source
+ tarballs?
+
+## Integrator Checklist
+
+### General
+
+* Is the exact code change to be merged available as an email on openvpn-devel
+ mailing list (note: this does not apply to the commit message since that needs
+ to be massaged anyway)?
+* If not, is the change to merge a trivial cherry-pick?
+
+### Commit message
+
+* Does it contain the pointer to the email on openvpn-devel?
+ This means a line `Message-Id: <actual msg id>` and a line
+ `URL: <url to mail-archive.com copy of the mail>`. If mail-archive.com
+ is not working, alternatively an URL to SourceForge's mail archive
+ is also acceptable.
+* Does it contain at least one `Acked-by` line from a known OpenVPN
+ core developer? Valid sources for `Acked-by` lines are mails on
+ the mailing list or +2 votes in Gerrit.
+* If the change is a security fix, does it contain a `CVE:` line in
+ the format `CVE: <Year>-<Number>`?
+* If the change fixes a Github issue, does it contain a line
+ `Github: OpenVPN/openvpn#<Number>`? (Other repositories might
+ also be referenced if applicable)
+* Have you considered adding a `Reported-by:` line? This is usual
+ for attribution for security issues but might be added for any
+ report where the reporter added significant value by his report.
+* If doing a cherry-pick, have you made sure to use `git cherry-pick -x`
+ to add a reference to the picked commit?
+
+#### Gerrit specific
+
+Additional checks if the change was reviewed and submitted via Gerrit.
+
+* Does it contain a correct `Gerrit URL:` line? (This is generally ensured
+ by using `gerrit-send-email.py`)
+* Is the `Change-Id:` line present and located in the last paragraph of the
+ message? (If not in the last paragraph, Gerrit will ignore it)
+
+### Automated tests
+
+* Has the change passed a full build run in buildbot? (Usually ensured via
+ Gerrit). This ensures the following points (which otherwise might need to
+ be considered separately):
+ * Build passes on multiple platforms (Linux, FreeBSD, Windows, macOS, other BSDs)
+ * Build passes with multiple compilers (GCC, Clang, MSVC, GCC MinGW)
+ * Build passes with multiple configurations (`--enable-small`, `--disable-management`,
+ etc.)
+ * Build passes with multiple versions of multiple SSL libraries (OpenSSL, mbedTLS)
+ * Unit tests pass in all these settings
+ * t\_server\_null, t\_client pass in all these settings
+ * t\_server passes
+ * Verified clang-format code formatting
+* If buildbot ignored the change that usually means that it did not match the
+ file match list (e.g. only changes in `.github/`). Consider whether that seems
+ correct.
+* Has the change passed a GHA run?
+* Do any of the tests actually excercise the code? If not, has someone other than the
+ submitter done any manual verification?
+
+### Backports
+
+* Is it clear to which branches the change should be applied?
+ * If it is a security fix, it should be applied to all branches that are
+ affected and that are not "unsupported"
+ (cf. https://community.openvpn.net/Pages/Supported%20versions)
+ * If it is a bugfix, it should be applied to all branches that are in
+ "Full stable support". Depending on the severity it might also be
+ applied to branches in "Old stable support".
+* Have you checked for all branches whether a trivial cherry-pick is
+ possible or whether a backport is required? If a backport is required
+ have you communicated this to the submitter?
@@ -67,6 +67,7 @@
.gitignore \
.gitattributes \
CONTRIBUTING.rst \
+ CODE_CHECKLIST.md \
PORTS \
README.cmake.md \
README.dco.md \