Thanks for writing formalising this!
There are some really minor nits below. You might want to consider renaming this to CONTRIBUTING.md to make sure that it renders nicely in tool GUIs (e.g., GitHub) that understand markdown.

CONTRIBUTING (Diff revision 1)

Typo

Show all issues

CONTRIBUTING (Diff revision 1)

Typo: Delete 'use'.

Show all issues

CONTRIBUTING (Diff revision 1)

It might be worth adding a paragraph to explain the why there are both reviewers and maintainers ackign changes here.

Show all issues

CONTRIBUTING (Diff revision 1)

Since we are switching to Gerrit, we should just state something along these lines: "You generally don't need to add these manually as they are added automatically by Gerrit."

Show all issues

CONTRIBUTING (Diff revision 1)

"Added automatically by Gerrit"

Show all issues

CONTRIBUTING (Diff revision 1)

We might want to specify that this is added automatically by a commit hook in git.

Show all issues

CONTRIBUTING (Diff revision 1)

We might want to move the second half of this paragraph to the description of signed-off-by above.

CONTRIBUTING (Diff revision 1)

The first slash in /refs/for/... isn't needed. I'm not sure if it breaks anything though.

Show all issues

Ship It!

Ship It!

Ship It!

Thanks for getting this in shape. Some minor things worth considering.

CONTRIBUTING.md (Diff revision 2)

This is quite defensive and negative at the moment.
Could we not make 1 and 2:

Share your work with others, so that they can benefit from new functionality.
Support the scientific principle by enabling others to evaluate your suggestions without having to guess what you did.

Show all issues

CONTRIBUTING.md (Diff revision 2)

I would think we should drop the s at the end.

Show all issues

CONTRIBUTING.md (Diff revision 2)

Is there a step where tests/regressions are added and/or updated, or is that perhaps too much detail for this doc?

Show all issues

1. Andreas Sandberg Feb. 23, 2017, 3:34 a.m. (Feb. 23, 2017, 3:34 a.m.)

   We probably shouldn't include that in the general contribution guide since users in general can't run the regressions (binary restrictions etc.). The code review system will integrate with a hosted Jenkins instance to automatically test that code is functionally correct, but there is little we can do automatically for stat diffs.

CONTRIBUTING.md (Diff revision 2)

I like Andreas' comments, but actually I don't think the original is negative either. Andreas 1,2 is probably a good reason to do it and the current 1,2 are the carrots of why you should.

Show all issues

CONTRIBUTING.md (Diff revision 2)

Where is the "Generate Password" option?  I don't see it after signing in.  Is this password unique to just pushing patches to gerrit and is it independent of your google account password?

Show all issues

1. Andreas Sandberg March 3, 2017, 6 a.m. (March 3, 2017, 6 a.m.)

   It's in the top bar when you navigate to https://gem5.googlesource.com/. The password is used to avoid storing and sending your normal account password (it's effectively a replacement for SSH keys) across when pushing code for review.

Several of us have discussed this document at AMD and we have a few more questions and comments below.
I assume reviewboard is the best place to continue this conversation since this patch has not been moved to Gerrit.

CONTRIBUTING.md (Diff revision 2)

Can we add the Gerrit testing before the code is posted for review?  Reviewers should know that the code passes before they review it.

Show all issues

1. Andreas Sandberg March 3, 2017, 6:01 a.m. (March 3, 2017, 6:01 a.m.)

   AFAIK, there is no easy way to get gerrit to run the regressions before publishing a review. One possibility would be to submit a draft review and wait for Jenkins to run and then publish the review. The problem with that approach is that (automatically) assigned reviewers typically get notifications for draft reviews. In my experience, this isn't a large problem.

CONTRIBUTING.md (Diff revision 2)

What happens when a maintainer has not been assigned to a region of code?  Is there a way even to guarantee that all existing portions of the code have a maintainer assigned?
It seems like we should say if there is a maintainer, then one must get their approval.  However in situations where one has not been assigned, we should skip this step.

Show all issues

1. Andreas Sandberg March 3, 2017, 6:01 a.m. (March 3, 2017, 6:01 a.m.)

   I think we need some way of tracking maintainers on a per file/directory basis. Linux has a similar system in place (see the MAINTAINERS file). In addition to component/architecture maintainers, I expect we'll end up with a set of core maintainers that are responsible for shared functionality and ACKs unmaintained bits.

CONTRIBUTING.md (Diff revision 2)

Restricting maintainer to be PMC member seems too restrictive.  When someone contributes large portions of the code (especially code that adds new features) the contributor will likely be the best person to maintain the code.

Show all issues

1. Andreas Sandberg March 3, 2017, 6:01 a.m. (March 3, 2017, 6:01 a.m.)

   Good point. I think we might need to move in this direction to avoid scalability issues going forwards.

Please go ahead with this and we can fine tune later. Could you also update http://gem5.org/Submitting_Contributions accordingly?

I went ahead and pushed this to the mainline this morning. Sorry I was so slow to do this. There are some outstanding issues, but we can modify the document to fix them as we go.
To follow up on Brad's comments about maintainers: We need to add a new MAINTAINERS file. For now, we'll assume that the PMC is responsible for everything, and we can add maintainers as we go. If someone has a suggestion for how to assign maintainers, let us know (probably in a new email chain).