trafficserver-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leif Hedstrom <zw...@apache.org>
Subject [IMPORTANT] Procedure around committing pull requests (and commits in general)
Date Fri, 10 Jul 2015 18:21:51 GMT
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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message