Commit Access

From gem5
Revision as of 11:35, 23 May 2014 by Atgutier (talk | contribs) (Bolded the keywords list to make it easier to find.)
Jump to: navigation, search

So, you have commit access to the gem5 repository. That's fantastic, we're glad that you're willing to help out. We're relatively light on process, but there are some important things that you need to know before you go changing the repository.

Short Summary

  1. Learn how to use Mercurial (HG) and Mercurial Queues (MQ)
  2. Break up your changes into meaningful, self contained pieces (and use MQ to help)
  3. Create meaningful commit messages and make sure that your name is on them
  4. Read your patches (hg diff, hg qdiff, hg export, and .hg/patches)
  5. Make sure everything compiles
  6. Make sure regressions pass
  7. Get code reviews (hg postreview)
  8. Make changes as requested and re-post if necessary
  9. Update your tree if any other changesets have been pushed during the review process (which is likely)
  10. Compile and run regressions again if you have changed your patch or updated your tree since steps 5-6
  11. Test different functionalities like cpu switching, checkpoint creation, restore if your patch might have any affect on those. Note that these functions are not tested by regression tests, except for cpu switching for the ARM architecture.
  12. Commit your changes/Finish your patches (hg commit or hg qfinish)
  13. Check what you're pushing before you push (hg outgoing)
  14. Contribute your changes (hg push)

Details

  • Learn how to use mercurial. If you don't know what you're doing, please ask questions and ask for help. The tool does take some getting used to, but breaking the tree is not something that is fun to clean up.
  • When you've got something that you want to commit, think about what it does and who it might affect. Is it self contained? Can it be broken up into more logical segments? M5 is a big project and there are a lot of people working on it. No one person knows everything, so we rely on the revision history and on comments to understand what is going on in the code. It is very important that you make change sets self contained. It is far easier to understand a series of ten 200 line changesets and what they do compared to a single 2000 change set that does a whole lot of stuff. This will also help you as a developer. If you keep your changesets small and self contained and you read them regularly, you will find your own bugs before you commit them.
  • You really really really should use Mercurial Queues (MQ). It makes this whole process so much easier. MQ does take a little bit to get used to, but I guarantee they make life easier for everyone. More info and a tutorial are available. Also, see the Managing Change in Your Local Repository for tips on keeping your local gem5 repo up-to-date with upstream changes.
  • Please don't forget to put commit messages on your patches. The -e, -l, or -m options to hg qref will allow you to do this (run hg help qref for details). Commit messages should have a keyword, followed by a colon followed by a single line of text summarizing the changeset, followed optionally by additional lines with further detail. The first line should be no more than 65 characters, since it's preceded by summary: in the brief hg log output. For example:
SCons: fixed the way the build system handled ISAs
Note the keyword and colon on the first line, and that it's < 65 chars.
Please keep your detailed commit comments to 80 columns.  These comments
will likely have to be wrapped manually, but that's not so bad a thing
to have to do.
  • The keyword should be one or more of the following separated by commas: base, ext, stats, sim, config, ruby, mem, arm, x86, arch, cpu, dev, scons, alpha, mips, power, sparc, kvm, tests, misc.
  • We want to be able to identify everyone who makes a change to the repository, so we need your name to show up in a consistent manner in the repository. Please dd this to your $HOME/.hgrc file before you start making changesets (hg commit or hg qfinish)
[ui]
username=Nathan Binkert <nate@binkert.org>
  • Please try to follow the Coding Style. They make search and replace much more effective. It also makes it easier for developers to follow code if it all has a similar look. I'm sure that there are things that you hate about the style. I've never worked on a project where everyone liked the style, unless they've all worked on the project for a long time and just gotten used to it.
  • Before you circulate your patch for review, ask yourself: Have I compiled this code for all ISAs? Have I run the regression tests that are relevant? (You should run at least the quick regressions and the long too if you think you're going to change any results.) You should avoid embarrassing yourself, annoying reviewers, or worst of all breaking the tree with trivial things that can be caught by these two steps.
  • Now that you've made small, self contained changesets, you should seek feedback from people. We have our own Reviewboard server for this process. You can use the Reviewboard Extension which provides the hg postreview command to send out your changesets for review. Some Tips:
    • If you want to post a review for the changeset at the tip, use "hg postreview -o"
    • If you want to update a review for the changeset at the tip, use "hg postreview -o -u -e <number>", where <number> is the review number on reviewboard.
    • Everything is easier if you are using Mercurial Queues as you can use "hg qpush" and "hg qpop" to get your diff to the tip and then use the postreview commands above.
  • Your reviews may request some changes; if so, make those and (unless the changes are trivial) re-post the modified patch for review, updating the existing review as described above.
  • Once the reviewers are satisfied with your change, make sure your change is based on the up-to-date head of the tree. If you're using mq, this is pretty trivial: hg qpop -a; hg pull -u; hg qpush (assuming the tree hasn't changed too much out from under you)
  • If you made any changes in the last two steps, recompile and re-run the quick regression tests. Even if your changes were totally trivial, you should at least recompile and run the "hello world" test to make sure you haven't done something dumb (speaking from experience).
  • If you're a new developer and don't have commit access, you'll have to get someone with commit access to push the patch to the repository on your behalf. If you do have commit access, follow these steps:
    • Now that you're ready to commit, you can hg qfinish the relevant patches to turn them into changesets.
    • Ok, so you're ready to push your changesets to the repository. One last step. Run hg outgoing. Are you about to push what you think you are? Do the commit messages all make sense? Ok, go ahead and hg push.

Review Process

If you have commit access the following patches can be pushed immediately (provided that you agree to change the patch as soon as possible if issues are raised):

  1. Patches that are small bug fixes (~10 lines)
  2. Additions to the documentation
  3. Updates to statistics

If your patch touches Ruby code, please get a review from Brad or Nilay before committing. If your patch touches ARM code please get a review from Ali or Andreas before committing.

A patch can be committed after it has been on the reviewboard for 10 days if no one has commented on it or asked for you to wait.

Retrieved from "http://repo.m5sim.org/wiki/index.php?title=Commit_Access&oldid=7720"