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,20 @@ 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.
- Anyone can merge after one positive review and with no obvious open questions and, merge ASAP so as not to leave approved PRs hanging.
lpantano marked this conversation as resolved.
Show resolved Hide resolved
- Exception in merges to master for releases (two reviews).
lpantano marked this conversation as resolved.
Show resolved Hide resolved
- Exception in pseudo PR review for first release (needs review from core or maintainer team).
Copy link
Member

Choose a reason for hiding this comment

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

Link to docs for first release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I cannot find exactly this? I found pipeline release but not first release

lpantano marked this conversation as resolved.
Show resolved Hide resolved
- If it's a major change then it's ok to request a second review - either the author or reviewer can do this.
lpantano marked this conversation as resolved.
Show resolved Hide resolved
- PRs that have approving reviews but open questions can be merged by the author after addressing the questions to a common-sense level of satisfaction.
lpantano marked this conversation as resolved.
Show resolved Hide resolved
- If the reviewer feels uneasy about this then they shouldn't leave an approval, but rather a request-changes.
- 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.
lpantano marked this conversation as resolved.
Show resolved Hide resolved
- Feature branches should be immediately deleted after merge. Ideally by selecting the new feature on GitHub repo settings
- Note: squash merging commits in a PR and merging after is fine, that is up to the individuals.
lpantano marked this conversation as resolved.
Show resolved Hide resolved

## 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