Skip to content

Commit

Permalink
website: Update the contributing.md for GitHub (#65)
Browse files Browse the repository at this point in the history 
* website: Update the contributing.md for GitHub

* website: Fix incorrect squash-and-merge link

Co-authored-by: Jason Lowe-Power <[email protected]>

* website: Move and expand 'Keeping your forked repo..."

This expands how to keep your forked and local repository up-to-date
(i.e., in-synic with the main gem5 repository).

It also moves it to it's own section.

* website: Fix 'stable' -> 'develop' typo

* website: 'recommend' -> 'strongly recommend' for local branches

* website: Minor changes to contributing.md

---------

Co-authored-by: Jason Lowe-Power <[email protected]>
  • Loading branch information
@BobbyRBruce @powerjg
BobbyRBruce and powerjg authored Jul 14, 2023
1 parent 7abf52d commit dced15a
Showing 1 changed file with 94 additions and 125 deletions.
219 changes: 94 additions & 125 deletions _pages/contributing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,23 @@ to the gem5 project.
## Determining what you can contribute

The easiest way to see how you can contribute to gem5 is to check our Jira
issue tracker: <https://gem5.atlassian.net>. From Jira you can check open
issues.
issue tracker: <https://gem5.atlassian.net> or GitHub issue tracker:
<https://github.com/gem5/gem5/issues>.

Browse these open issues and see if there are any which you are capable of
handling. When you find a task you are happy to carry out, verify no one else
is presently assigned, then leave a comment asking if you may assign yourself
this task (this will involve creating a Jira account). Though not mandatory, we
this task. Though not mandatory, we
advise first-time contributors do this so developers more familiar with the
task may give advice on how best to implement the necessary changes.

Once a developers has replied to your comment (and given any advice they may
have), you may officially assign yourself the task. After this you should
change the status of the task from `Todo` to `In progress`. This helps the gem5
have), you may officially assign yourself the task. This helps the gem5
development community understand which parts of the project are presently being
worked on.

**If, for whatever reason, you stop working on a task, please unassign
yourself from the task and change the task's status back to `Todo`.**
yourself from the task.**

## Obtaining the git repo

Expand All @@ -44,16 +43,28 @@ exclusively.**

To pull the gem5 git repo:

```Shell
```sh
git clone https://github.com/gem5/gem5
```

In order to make changes to this repo, fork the gem5 repo from the link
above into your own repository.
If you wish to use gem5 and never contribute, this is fine. However, to
contribute, we use the [GitHub Pull-Request model](https://docs.github.com/en/pull-requests), and therefore recommend [Forking the gem5 repository](https://docs.github.com/en/get-started/quickstart/fork-a-repo) prior to contributing.

### Forking

Please consult the [GitHub documentation on Forking a GitHub repository](https://docs.github.com/en/get-started/quickstart/fork-a-repo).
As we will be working atop the `develop` branch, please ensure you Fork all the repository's branches, not just the `stable` branch.

This will create your own forked version of the gem5 repo on your own GitHub account.
You may then obtain it locally using:

```sh
git clone https://github.com/{your github account}/gem5
```

### stable / develop branch

By default, the git repo will have the `stable` branch checked-out. The
When cloned the git repo will have the `stable` branch checked-out by default. The
`stable` branch is the gem5 stable release branch. I.e., the HEAD
of this branch contains the latest stable release of gem5. (execute `git tag`
on the `stable` branch to see the list of stable releases. A particular
Expand All @@ -62,24 +73,24 @@ release may be checked out by executing `git checkout <release>`). As the
should not develop changes on top of the `stable` branch** they should instead
**develop changes on top of the `develop` branch**.

To checkout the `develop` branch:
To switch to the `develop` branch:

```Shell
git checkout --track origin/develop
```sh
git switch develop
```

Changes may be made on this branch to incorporate changes assigned to yourself.
The develop `branch` is merged into the `stable` branch upon a gem5 release.
Therefore, any changes you make exist on the develop branch until the next release.

As the develop branch is frequently updated, regularly obtain the latest
`develop` branch by executing:
We strongly recommend creating your own local branches to do changes.
The flow of development works best if `develop` and `stable` are not modified directly.
This helps keep your changes organized across different branches in your forked repository.
The following example will create a new branch, from `develop`, called `new-feature`:

```
git pull --rebase
```sh
git switch -c new-feature
```

Conflicts may need resolved between your local changes and new changes on the
`develop` branch.

## Making modifications

### C/CPP
Expand Down Expand Up @@ -159,7 +170,7 @@ Then run on modified/added python files using:
black <files/directories>
```

For varibale/method/etc. naming conventions, please follow the [PEP 8 naming
For variable/method/etc. naming conventions, please follow the [PEP 8 naming
convention recommendations](
https://peps.python.org/pep-0008/#naming-conventions). While we try our best to
enforce naming conventions across the gem5 project, we are aware there are
Expand All @@ -181,29 +192,26 @@ pre-commit install
```

Once installed pre-commit will run checks on modified code prior to running the
`git commit` command (see [our section on commiting](#committing) for more
details on commiting your changes). If these tests fail you will not be able to
`git commit` command (see [our section on committing](#committing) for more
details on committing your changes). If these tests fail you will not be able to
commit.

These same pre-commit checks are run as part of GitHub's CI checks (those
which must pass the status checks required for a change to be
incorporated into the develop branch). It is therefore recommended that
developers install pre-commit to catch style errors early.

**Note:** As of the v22.0 release, the pre-commit hook is only available on the
develop branch.
These same pre-commit checks are run as part our CI checks (those
which must pass in order for a change to be merged into the develop branch). It
is therefore strongly recommended that developers install pre-commit to catch
style errors early.

## Compiling and running tests

The minimum criteria for a change to be submitted is that the code is
compilable and the test cases pass.

The following command both compiles the project and runs our system-level
checks:
The following command both compiles the project and runs our "quick"
system-level checks:

```Shell
```sh
cd tests
python main.py run
./main.py run
```

**Note: These tests can take several hours to build and execute. `main.py` may
Expand All @@ -212,22 +220,19 @@ be run on multiple threads with the `-j` flag. E.g.: `python main.py run

The unit tests should also pass. To run the unit tests:

```Shell
```sh
scons build/NULL/unittests.opt
```

To compile an individual gem5 binary:

```Shell
scons build/{ISA}/gem5.opt
```sh
scons build/ALL/gem5.opt
```

where `{ISA}` is the target ISA. Common ISAs are `ARM`, `MIPS`, `POWER`,
`RISCV`, `SPARC`, and `X86`. So, to build gem5 for `X86`:

```Shell
scons build/X86/gem5.opt
```
This compiles a gem5 binary containing "ALL" ISA targets. For more information
on building gem5 please consult our [building documentation](
/documentation/general_docs/building).

## Committing

Expand Down Expand Up @@ -279,66 +284,67 @@ Jira Issue: https://gem5.atlassian.net/browse/GEM5-186
If you feel the need to change your commit, add the necessary files then
_amend_ the changes to the commit using:

```
```sh
git commit --amend
```

This will give you opportunity to edit the commit message.

## Pushing to GitHub
You may continue to add more commits as a chain of commits to be included in the pull-request.
However, we recommend that pull-requests are kept small and focused.
For example, if you wish to add a different feature or fix a different bug, we recommend doing so in another pull requests.

Pushing to GitHub will allow others in the gem5 project to review the change to
be fully merged into the gem5 source.
## Keeping your forked and local repositories up-to-date

To start this process, execute the following in your forked repository:
While working on your contribution, we recommend keeping your forked repository in-sync with the source gem5 repository.
To do so, regularly [Sync your fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork).
This can be done via the GitHub web interface and, if so, you should `git pull` on top of your local `stable` and `develop` branches to ensure your local repository is in-sync.
To do so from the command line:

```
git push origin
```sh
git fetch upstream # Obtain the latest from the gem5 repo.
git switch develop # Switch to the develop branch.
git merge upstream/develop # Merge the latest changes into the develop branch.
git push # Push to develop to your forked repo.
git switch stable # Switch to the stable branch.
git merge upstream/stable # Merge the latest changes into the stable branch.
git push # Push the changes to stable to your forked repo.
```

In order to have these changes reviewed, now go onto GitHub and create a
pull request from your forked repo, onto the gem5-website repo.
As our local branch work atop the `develop` branch, once we've synced our forked repository, we can rebase our local branch on top of the `develop` branch.
Assuming our local branch is called `new-feature`:

## Code review
```sh
git switch develop # Switching back to the develop branch.
git pull # Ensuring we have the latest from the forked repository.
git switch new-feature # Switching back to our local branch.
git rebase develop # Rebasing our local branch on top of the develop branch.
```

Now, at <https://github.com/gem5/gem5/pulls>, you can view the
change you have submitted. We suggest that, at this stage, you mark the
corresponding Jira issue as `In Review`. Adding a link to the change on
GitHub as a comment to the issue is also helpful.
Conflicts may need resolved between your branch and new changes.

Through the GitHub pull request we strongly advise you add reviewers.
GitHub will automatically notify those you assign. The "maintainers" of the
components you have modified should be added as reviewers. These should
correspond to the tags you included in the commit header. **Please consult
[MAINTAINERS.yaml](
https://github.com/gem5/gem5/blob/stable/MAINTAINERS.yaml) to
see who maintains which component**. As an example, for a commit with a header
of `tests,arch : This is testing the arch component` then the maintainers for
both `tests` and `arch` should be included as reviewers.
## Pushing and creating a pull request

Reviewers will then review this change. There are two categories which the commit
shall be evaluated: "Code-Review", and the CI-tests workflow.
Once you have completed your changes locally, you can push to your forked gem5 repository.
Assuming the branch we are working on is `new-feature`:

Any person can review your pull request. They can either approve your changes,
or make suggestions on what needs to be fixed before approval can be given.
```sh
git switch new-feature # Ensure we are on the 'new-feature' branch.
git push --set-upstream origin new-feature
```

Upon the creation of a pull request, our continuous integration system will process
the change. You can see what tests will be run in `.github/workflows/ci-tests.yaml`
Now, via the GitHub web interface, you can [create a pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) of your changes from your forked repository's branch into the gem5 `develop` branch.

If this executes successfully (i.e. the project builds and the tests pass) the
continuous integration system will pass the status checks within GitHub.
## Passing the checks

However, for first-time contributors, someone will first need to review,
and approve your pull request before the continous integration tests
begin.
Once you have created a pull request, the gem5 Continuous Integration (CI) tests will run.
These run a series of checks to ensure your changes are valid.
These must pass before your changes can be merged into the gem5 `develop` branch.

In order for a pull request to be merged, one of the Maintainers of the
gem5 repo will have to hit the merge button. This allows for final checks
to be done, ensuring the quality of code entering the gem5 codebase.
In addition to the CI tests, your changes will be reviewed by the gem5 community.
Your pull-request must have the approval of at least one community member prior to being merged.

For non-trivial changes, it is not unusual for a change to receive feedback
from reviewers that they will want incorporated before giving the commit a
score necessary for it to be merged. This leads to an iterative process.
Once your pull-request has passed all the CI tests and has been approved by at least one community member, it will be merged a gem5 Project Maintainer will do a [Squash and Merge](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits) on the pull-request.

### Making iterative improvements based on feedback

Expand All @@ -347,45 +353,8 @@ these comments and answer these questions. **All communications between
reviewers and contributors should be done in a polite manner. Rude and/or
dismissive remarks will not be tolerated.**

When you understand what changes are required, using the same workspace as
before, make the necessary modifications to the gem5 repo, and amend the
changes to the commit:

```Shell
git commit --amend
```

Then push the new changes to GitHub:

```Shell
git push origin
```

When your new change is uploaded via the `git push` command, the reviewers will
re-review the change to ensure you have incorporated their suggested
improvements. The reviewers may suggest more improvements and, in this case,
you will have to incorporate them using the same process as above. **This
process is therefore iterative, and it may therefore take several cycles until
the patch is in a state in which the reviewers are happy**. Please do not
be deterred, it is very common for a change to require several iterations.

## Submit and merge

Once this iterative process is complete. The patch may be merged. This is done
via GitHub (Simply click `Submit` within the relevant GitHub page).

As one last step, you should change the corresponding Jira issue status to
`Done` then link the GitHub page as a comment on Jira as to provide evidence
that the task has been completed.

Stable releases of gem5 are published three times per year. Therefore, a change
successfully submitted to the `develop` branch will be merged into the `stable`
branch within three to four months after submission.

## gem5 Bootcamp 2022
When you understand what changes are required make amendments to the pull
request by adding patches to the same branch and then pushing to the forked repository.

As part of [gem5's 2022 Bootcamp](/events/boot-camp-2022), contributing to gem5
was taught as a tutorial. Slides for this tutorial can be found [here](
https://ucdavis365-my.sharepoint.com/:p:/g/personal/jlowepower_ucdavis_edu/EQLtRAKI94JKjgk5pBmJtG8B3ssv9MaR0a2i92G0TwHK8Q?e=KN3NIppm2kg&action=embedview&wdbipreview=true).
A video recording of this tutorial can be found [here](
https://www.youtube.com/watch?v=T67wzFd1gVY).
Once pushed to the forked repository, the pull request will automatically update with your changes.
A reviewer will then review your changes and, if necessary, ask for further changes, or approve your pull-request.

0 comments on commit dced15a

Please sign in to comment.