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.

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

add draft to guidelines from slack discussion #2780

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ The main steps involved in adding a new nf-core pipeline covered below are:
2. [Contribution overview](#contribution-overview)
3. [Testing](#testing)
4. [Patching bugs](#patching-bugs)
5. [Getting help](#getting-help)
6. [Pipeline contribution conventions](#pipeline-contribution-conventions)
5. [PR review guidelines](#pr-review-guidelines)
6. [Getting help](#getting-help)
7. [Pipeline contribution conventions](#pipeline-contribution-conventions)

## Join the community

Expand All @@ -42,7 +43,7 @@ If you'd like to write some code for an nf-core pipeline, the standard workflow
2. [Fork](https://help.github.com/en/github/getting-started-with-github/fork-a-repo) the pipeline repository to your GitHub account
3. Make the necessary changes / additions on `dev` branch by using [git checkout](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork) within your forked repository following [pipeline conventions](#pipeline-contribution-conventions)
4. Use `nf-core pipelines schema build` and add any new parameters to the pipeline JSON schema (requires [nf-core tools](https://github.com/nf-core/tools) >= 1.10).
5. Submit a Pull Request against the `dev` branch and wait for the code to be reviewed and merged
5. Submit a Pull Request against the `dev` branch and wait for the code to be reviewed and merged. See below guidelines for this.

If you're new to working with git, you can view the [GitHub pull requests documentation](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests) or other [excellent `git` resources](https://try.github.io/) to get started.

Expand Down Expand Up @@ -80,6 +81,22 @@ Only in the unlikely and regretful event of a release happening with a bug.
- Fix the bug, and bump version (X.Y.Z+1).
- A PR should be made on `master` from patch to directly resolve this particular bug.

## PR review guidelines

- Authors/Reviewers can request help for review at nf-core Slack `#request-review` channel.
- We encourage collaboration in PR, other contributors are welcome to fix linting errors (you can mention relevant bot commands) or make improvements to the code if necessary.
- Authors/Reviewers should merge after one positive review and with no obvious open questions.
- Exception: merges to master for releases requires **two** reviews.
- Exception: pseudo PR reviews for a first release needs **review from core or maintainer team**.
- If it's there is a major change in any type of PR (from modules to pipelines) then it's ok to request a second review - either the author or reviewer can do this.
- PRs that have approving reviews but remaining open questions can be merged by the author after addressing the questions to a common-sense level of satisfaction.
- If the reviewer feels uneasy about this then they shouldn't leave an approval, but rather a request-changes.
- **request changes** reviews can be [dismissed](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/dismissing-a-pull-request-review) by the PR author if considered out of date and resolved.
- We prefer to use merge commits to merge. This is to avoid merge conflicts when multiple people are pulling with overlapping feature branches.
- We prefer verbose commit histories but easy merges.
- Feature branches should be immediately deleted after merge. Ideally by selecting the new feature on GitHub repo settings
- Note: squashing commits in a PR and merging after is fine, that is up to the individuals.

## Getting help

For further information/help, please consult the usage documentation for the particular pipeline and don't hesitate to get in touch on the nf-core Slack `#<pipeline-name>` channel. If you are not already a member you can ([join our Slack here](https://nf-co.re/join/slack)).
Expand Down
Loading