Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributor guide update #787

Merged
merged 3 commits into from
Dec 1, 2024
Merged

Contributor guide update #787

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9225123
First draft of updated contributor's guide
tennlee Nov 28, 2024
5090262
Further updates to the contributor's guide
tennlee Nov 28, 2024
cf69b4d
Update PR based on feedback to include information on fork syncing
tennlee Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions docs/contributing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,23 @@ Please submit bug reports, feature requests and feedback as issues in GitHub: [h

Please see the information provided in [SECURITY.md](SECURITY.md)

## Creating Your Own Fork of `scores` for the First Time

Unless you are an advanced Git user, we would recommend you follow this process:

1. First (i.e. **before** cloning the `scores` repository) create your own fork using the GitHub web user interface.
2. Clone **your fork**. (Do not clone github.com/nci/scores).
3. Immediately create a new local branch, with a command such as `git checkout -b branch_name`.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a line to periodically sync your fork

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could possibly fit into the section just below

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a commit to the PR to add some information on this to the PR workflow. Let me know if that seems sufficient.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - In addition it may be good to encourage people to sync their fork before they create a new feature branch to minimise the amount of rebasing etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While that's good advice, I feel like it may be stepping over into a more detailed guide that we don't really have the ability to provide. Ideally, it would be nice to link to some general resource that hopefully already exist out there. Feel free to do some looking around and raise a new PR if you find some good ones.

## Workflow for Submitting Pull Requests

Prior to developing a pull request, it may be a good idea to create a [GitHub issue](https://github.com/nci/scores/issues) to capture what the pull request is trying to achieve, any pertinent details, and (if applicable) how it aligns to the roadmap. Otherwise, please explain this in the pull request.

To submit a pull request, please use the following workflow:
1. create a fork of the `scores` repository,
2. create a feature branch on your fork,
3. keep your feature branch rebased and up-to-date with the `scores` develop branch,
4. when ready, submit a pull request to the develop branch of `scores`.

1. Ensure you are working on a new feature branch in **your fork** (see [the section above](#creating-your-own-fork-of-scores-for-the-first-time)).
2. Keep your feature branch rebased and up-to-date with the develop branch of `scores`.
3. When ready, submit a pull request to the develop branch of github.com/nci/scores.

To help disambiguate branches, some contributors like to prefix their branch names with a short numerical indentifier. This is up to the contributor and any approach to branch naming is welcome.

Expand Down Expand Up @@ -110,6 +118,18 @@ To automate linter and fixer checks this project uses `pre-commit` which is set
pre-commit install -t pre-commit -t pre-push
```

## Pull Request Etiquette

In general, the originator of a pull request will be the person who does all the coding work, including responding to feedback from others. Typically, feedback will be provided in the form of comments or code suggestions made through the GitHub web user interface.

Sometimes, it may be pragmatic for the package maintainers to make or request a more complex code change. This typically occurs when a complex Git operation is needed to keep a pull request (PR) up to date, to resolve conflicts, or to make changes where the originator of the PR does not know how to proceed. It can also occur in the final stages of a PR lifecycle if there are small tidyups needed and time is a factor.

Not every possibility can be accounted for, and the package maintainers will (if needed) push code directly to a PR to get something over the line. However, special circumstances aside, the maintainers will first leave a comment on the PR asking how the originator would like to proceed, so that there are no surprises.

Most of these kinds of code change can also be handled as a PR by the reviewer onto the fork of the originator. This is slightly slower (i.e. does not take effect immediately), but allows for more control by the originator. This is probably most developer's general preference.

In short - once you have made a PR, the maintainers may then take it, modify it, or include it as-is. However, every effort will be made to communicate about that process and make sure that the originator of the PR is happy with any modifications made.

## Review Processes

All pull requests will undergo a review process prior to being included in the develop branch in order to ensure both the coding and the scientific validity of changes.
Expand Down