[PATCH v1] Start a new document CODE_CHECKLIST

From: Frank Lichtenheld <[email protected]>

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 <[email protected]>
Acked-by: Razvan Cojocaru <[email protected]>
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 1 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <[email protected]>

        
diff --git a/CODE_CHECKLIST.md b/CODE_CHECKLIST.md
new file mode 100644
index 0000000..3c61ee3
--- /dev/null
+++ b/CODE_CHECKLIST.md
@@ -0,0 +1,112 @@
+# 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 <[email protected]>` ?
+
+### 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?
+
+## 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?