Authors: Jason Lowe-Power
Andreas Sandberg
Steve Reinhardt
+ Bobby R. Bruce
If you've made changes to gem5 that might benefit others, we strongly encourage
you to contribute those changes to the public gem5 repository. There are
how to create code changes, upload your changes, have your changes
reviewed, and finally push your changes to gem5. More information can be found
from the following sources:
- * http://gem5.org/Submitting_Contributions
+ * http://gem5.org/contributing
* https://gerrit-review.googlesource.com/Documentation/index.html
* https://git-scm.com/book
|
|
v
- +------+------+
- | Post review |
- +------+------+
- |
- v
- +--------+---------+
- | Wait for reviews | <--------+
+ +-------------+
+ | Run tests |<--------------+
+ +------+------+ |
+ | |
+ | |
+ v |
+ +------+------+ |
+ | Post review | |
+ +------+------+ |
+ | |
+ v |
+ +--------+---------+ |
+ | Wait for reviews | |
+--------+---------+ |
| |
| |
community. Additionally, the maintainer for that part of the code must sign off
on it.
+Contributing long-lived feature branches
+----------------------------------------
+Oftentimes users or institutions add features that are necessarily complex,
+and require many changes on long-lived feature branches. In this case,
+maintaining a perfect history where all changes work individually is infeasible.
+When contributing long-lived feature branches back to gem5's public repository
+users may merge entire long-lived branches into a single changeset and contribute
+their code back as long as 1) the changes have been reviewed by the maintainer
+2) the maintainer agrees to allow such a change, and 3) the changes are passing
+the public tests. Changes that affect common code (outside of a specific
+maintainer's purview) will still need to follow the standard gem5 protocol.
+
+
Cloning the gem5 repo to contribute
===================================
https://gem5.googlesource.com/.
To clone the master gem5 repository:
- > git clone https://gem5.googlesource.com/public/gem5
+```
+ git clone https://gem5.googlesource.com/public/gem5
+```
Other gem5 repositories
-----------------------
certifies the contribution is made under the terms of the Developer Certificate
of Origin (DCO) [https://developercertificate.org/].
+If your change relates to a [Jira Issue](https://gem5.atlassian.net), it is
+advised that you provide a link to the issue in the commit message (or messages
+if the Jira Issue relates to multiple commits). Though optional, doing this
+can help reviewers understand the context of a change.
+
It is imperative that you use your real name and your real email address in
both tags and in the author field of the changeset.
and resubmit. The following is a useful git command to update the most recent
commit (HEAD).
- > git commit --amend
+```
+ git commit --amend
+```
+
+Running tests
+=============
+
+Before posting a change to the code review site, you should always run the
+quick tests!
+See TESTING.md for more information.
Posting a review
================
Push change to gerrit review
----------------------------
- > git push origin HEAD:refs/for/master
+```
+ git push origin HEAD:refs/for/master
+```
Assuming origin is https://gem5.googlesource.com/public/gem5 and you want to
push the changeset at HEAD, this will create a new review request on top of the
master branch. More generally,
- > git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
+```
+ git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
+```
See https://gerrit-review.googlesource.com/Documentation/user-upload.html for
more information.
--------------------------
The first time you push a change you may get the following error:
- > remote: ERROR: [fb1366b] missing Change-Id in commit message footer
- > ...
+```
+ remote: ERROR: [fb1366b] missing Change-Id in commit message footer
+ ...
+```
Within the error message, there is a command line you should run. For every new
clone of the git repo, you need to run the following command to automatically
insert the change id in the the commit (all on one line).
- > curl -Lo `git rev-parse --git-dir`/hooks/commit-msg
- https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x
- `git rev-parse --git-dir`/hooks/commit-msg
+```
+ curl -Lo `git rev-parse --git-dir`/hooks/commit-msg \
+ https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; \
+ chmod +x `git rev-parse --git-dir`/hooks/commit-msg
+```
If you receive the above error, simply run this command and then amend your
changeset.
- > git commit --amend
+```
+ git commit --amend
+```
-Push change to gerrit as a draft
---------------------------------
+Push change to gerrit as a draft/private
+----------------------------------------
+
+See https://gerrit-review.googlesource.com/Documentation/intro-user.html#private-changes
+for details on private gerrit changes.
- > git push origin HEAD:refs/drafts/master
+```
+ git push origin HEAD:refs/for/master%private
+```
+
+Once you have pushed your change as "private", you can log onto [gerrit]
+(https://gem5-review.googlesource.com) and once you're happy with the commit
+click the "unmark private" which may be hidden in the "more options" dropdown
+in the upper right corner.
Push change bypassing gerrit
-----------------------------
Only maintainers can bypass gerrit review. This should very rarely be used.
- > git push origin HEAD:refs/heads/master
+```
+ git push origin HEAD:refs/heads/master
+```
Other gerrit push options
-------------------------
information.
https://gerrit-review.googlesource.com/Documentation/user-upload.html
+Branches
+========
+
+By default, contributions to gem5 should be made on the master branch. Branches
+are generally discouraged due to their tendency to bloat git repositories with
+abandoned code. However, the creation of branches is permitted for development
+of a specific feature or improvement if one or more of the following criteria
+are met:
+
+1. The feature/improvement is likely to be of a large size, consisting of many
+commits, with little logic in these commits being contributed separately.
+2. The feature/improvement will be developed over a long period of time.
+3. There is sufficient reason that a feature/improvement should not be merged
+into the master branch yet (e.g., the feature/improvement is not intended for
+the next release but a future release).
+
+If a branch is required it can only be created by a project maintainer.
+Therefore, if a gem5 contributor desires a separate branch for their work, they
+should request one from the maintainer of the component the work relates to
+(see MAINTAINERS for the list of maintainers and the components they are
+responsible for). **The maintainer shall use their discretion to determine
+whether the creation of a branch is necessary**. If approved, the maintainer
+shall create the branch which the contributor may then use.
+
+Development on a branch within Gerrit functions in exactly the same way as
+contributing to the master branch. When contributors to a branch are satisfied,
+they should create a merge commit into the master branch. The maintainer should
+then be notified that the branch they created can now be deleted.
+
+**Abandonment of changes within branches may result in these branches being
+removed from the repository. All branches within a repo should be under active
+development.**
Reviewing patches
=================
the patch can be merged. The patch will receive a +1 if gem5 builds and
runs, and it will receive a +2 if the stats match.
* Style-Check: This is automatically generated and tests the patch against the
- gem5 code style (http://www.gem5.org/Coding_Style). The patch must receive a
- +1 from the style checker to be pushed.
+ gem5 code style
+ (http://www.gem5.org/documentation/general_docs/development/coding_style/).
+ The patch must receive a +1 from the style checker to be pushed.
Note: Whenever the patch creator updates the patch all reviewers must re-review
the patch. There is no longer a "Fix it, then Ship It" option.
can simply push the changeset again to the same Gerrit branch to update the
review request.
- > git push origin HEAD:refs/for/master
-
-Note: If you have posted a patch and don't receive any reviews, you may need to
-prod the reviewers. You can do this by adding a reply to your changeset review
-on gerrit. It is expected that at least the maintainer will supply a review for
-your patch.
+```
+ git push origin HEAD:refs/for/master
+```
Committing changes
==================
Once a patch meets the above criteria, the submitter of the patch will be able
to merge the patch by pressing the "Submit" button on Gerrit. When the patch is
submitted, it is merged into the public gem5 branch.
+
+Review moderation and guidelines
+--------------------------------
+
+Once a change is submitted, reviewers shall review the change. This may require
+several iterations before a merge. Comments from reviewers may include
+questions, and requests for alterations to the change prior to merging. The
+overarching philosophy in managing this process is that there should be
+politeness and clear communication between all parties at all times, and,
+whenever possible, permission should be asked before doing anything that may
+inconvenience another party. Included below are some guidelines we expect
+contributors and reviewers to follow.
+
+ * In all forms of communication, contributors and reviewers must be polite.
+ Comments seen as being needlessly hostile or dismissive will not be
+ tolerated.
+ * Change contributors should respond to, or act upon, each item of feedback
+ given by reviewers. If there is disagreement with a piece of
+ feedback, a sufficiently detailed reason for this disagreement should
+ be given. Polite discussion, and sharing of information and expertise
+ is strongly encouraged.
+ * Contributors are advised to assign reviewers when submitting a change.
+ Anyone who contributes to gem5 can be assigned as a reviewer. However,
+ all changes must be accepted by at least one maintainer prior to a
+ merge, ergo assigning of at least one maintainer as a reviewer is
+ strongly recommended. Please see MAINTAINERS for a breakdown of
+ gem5 maintainers and which components they claim responsibility for.
+ Maintainers should be chosen based on which components the change is
+ targeting. Assigning of reviewers is not strictly enforced, though not
+ assigning reviewers may slow the time in which a change is reviewed.
+ * If a contributor posts a change and does not receive any reviews after two
+ working days (excluding regional holidays), it is acceptable to "prod"
+ reviewers. This can be done by adding a reply to the changeset review
+ (e.g., "Would it be possible for someone to review my change?"). If the
+ contributor has yet to assign reviewers, they are strongly advised to do so.
+ Reviewers will get notified when assigned to referee a change.
+ * By default, the original contributor is assumed to own a change. I.e.,
+ they are assumed to be the sole party to submit patchsets. If someone
+ other than the original contributor wishes to submit patchsets to a
+ change on the original contributor's behalf, they should first ask
+ permission. If two working days pass without a response, a patchset may be
+ submitted without permission. Permission does not need to be asked to submit
+ a patchset consisting of minor, inoffensive, changes such a typo and format
+ fixes.
+ * Once a change is ready to merge, it enters a "Ready to Submit" state. The
+ original contributor should merge their change at this point, assuming they
+ are content with the commit in its present form. After two working days, a
+ reviewer may message a contributor to remind them of the change being in a
+ "Ready to Submit" state and ask if they can merge the change on the
+ contributors behalf. If a further two working days elapse without a
+ response, the reviewer may merge without permission. A contributor may keep
+ a change open for whatever reason though this should be communicated to the
+ reviewer when asked.
+ * After a month of inactivity from a contributor on an active change, a
+ reviewer may post a message on the change reminding the submitter, and
+ anyone else watching the change, of its active status and ask if they are
+ still interested in eventually merging the change. After two weeks of no
+ response the reviewer reserves the right to abandon the change under the
+ assumption there is no longer interest.
+ * The final arbiter in any dispute between reviewers and/or contributors
+ is the PMC (PMC members are highlighted in MAINTAINERS). Disputes requiring
+ intervention by the PMC are undesirable. Attempts should be made to resolve
+ disagreements via respectful and polite discourse before being escalated to
+ this level.
projects / gem5.git / blobdiff