2 What we look for when reviewing and merging pull requests
Claudio Cambra edited this page 2022-02-11 17:31:05 +01:00

Is there a good reason for making this change?

Change for the sake of change is never a good thing! When submitting a pull request, please provide good reasoning for your changes. This will make it much more likely for reviewers to eventually approve your PR.

Does your pull request description adequately explain what changes are being made?

An explanation of what your PR changes will make the lives of reviewers much easier. We don't need you to write extensive documentation about your changes, but a basic overview will be much appreciated.

Are there any screenshots showing changes to the user interface?

It is often very difficult to tell what has been changed visually just by looking at the code. If you are modifying any of the UI components of the desktop client, please provide a screenshot!

For user interface changes, has a member of the design team approved the changes?

The designers are the most knowledgeable regarding what makes a user interface usable, accessible, and attractive. Whenever a change is made to the UI, we always seek the feedback of the designers.

What is the status of the pull request? Is it ready for review or is it still a draft?

If your pull request is marked as a draft, it is likely to be deprioritised over PRs that are ready. Once you are finished making changes to your pull request, and it is ready for review, make sure to mark it as ready!

Does new/changed code follow our coding style?

Nextcloud's desktop client team follows coding style guidelines that make our code readable and maintainable. Please ensure that your code conforms to these guidelines.

Do your changes pass the continuous integration pipeline and pass the desktop client's unit tests?

Testing is an essential part of ensuring the continued stability of the desktop client, and it is mandatory that new code does not break existing tests and CI pipelines.

Is your code covered by unit tests?

Unit tests are not always a required addition to a PR, but their inclusion will always accelerate and improve the review process.

Do interactions with reviewers and the wider community follow the code of conduct?

The Nextcloud community has a code of conduct that we expect all community members to follow. Breaking the code of conduct at any stage of the PR submission or review process will stop us from approving it.

Have at least two Nextcloud developers approved the changes?

All new code merges for the desktop client require at least two members of the desktop client team to approve the changes. This helps us ensure that new code is stable and efficient.

Good luck with your pull request!