Hi all,
since we’re seeing a pretty significant increase in pull requests (primarily
from github), I’d like to remind everyone about some guidelines for committing
and testing. This applies to both commits you make on someone else’s pull
request, as well as to your own commits.
1. Make sure to review the code, particularly if it’s someone else’s code that
you are committing (merging). If you are uncertain, it’s always ok to ask for
another pair of eyeballs to take a look. Remember, we do commit then review,
and it’s everyones responsibility to review code.
2. Make sure to run all tests before committing. This means at a *minimum*:
- sudo traffic_server -R 1
- make test #from top of build tree
Neither should fail, both are mandatory to always succeed. For extra bonus
and good karma, run tsqa as well (although, that is not as straightfoward as
we’d like, yet).
3. Not required, but definitely recommended for large changes: Make a debug
build, and run all tests in debug mode. Additionally, I highly recommend
everyone has a CentOS6 VM to test builds on, this is our minimum required
“platform”.
4. If you are adding new features, or modifying existing features, adding (or
modifying) tests is definitely a huge win. Eventually, we might even make it
mandatory (but that’s a different topic).
5. Before you commit, make sure the code is properly formatted using
clang-format. This is easiest done with a simple “make clang-format”. Make sure
you run / use the correct clang-format binary, from
https://bintray.com/apache/trafficserver/clang-format-tools/view
<https://bintray.com/apache/trafficserver/clang-format-tools/view> . In
addition, there are tools there (such as git clang-format) that are also
helpful. I’m working on updated the Docs on the Wiki for these processes as
well.
6. Before you commit, check the CI status, at
https://ci.trafficserver.apache.org <https://ci.trafficserver.apache.org/>. If
there are currently build failures on master, I’d strongly recommend that you
defer committing, and instead help fixing the build errors. Even just figuring
out what failed, and asking the committer to fix it is a bonus. Piling on more
code to an already busted build doesn’t help anyone.
7. Make sure to update any documentation, Jira’s (fix versions, resolutions
etc.) and close the github pull request (if applicable).
8. Check your emails and CI for build errors *after* you commit. Emails from
Jenkins are flaky at best, so it’s always a good idea to eyeball the site once
in a while. It doesn’t take long to get a good idea of what the status is.
Finally, I’d ask everyone to consider joining the issues@trafficserver mailing
list, and monitor new and resolved Jira’s, as well as general build errors from
Jenkins.
Sincerely,
— leif
