> On Jul 10, 2015, at 11:21 AM, Leif Hedstrom <zw...@apache.org> wrote: > > 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
The ./ci/regression script does this (as well as verify out-of-tree builds). > 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 >
