Get involved in Nextcloud by reviewing pulls!

Originally published at: Get involved in Nextcloud by reviewing pulls! - Nextcloud

Starting to contribute to Nextcloud isn’t entirely trivial. Openhub calculates that all of Nextcloud and its apps consists of almost 5.5 millions lines of code, mostly PHP but also significant parts of JavaScript and CSS and, for the clients, C++, Java and Objective-C. Of course, to contribute you don’t have to know all of that but there is a barrier nonetheless. Today we’d like to make a case for reviewing code as a way to get involved!

What is reviewing?

Nextcloud has a mandatory review process: no code can be merged until at least two other people besides the developer of the initial code have had a look and agreed that the quality and style are up to our standards. Besides the human review a series of automated tests also runs over each proposal code inclusion (a pull request). Here is an example of a pull request with reviews having taken place:

a quick review turn-around

What you see is, first, that pull request number 4316 was created by MorrisJobke and consists of 1 commit and has 2 changed files. You can click on those to see the changes that were made.

He added a ‘to review’ label, so others knew he was looking for feedback, and added it to the Nextcloud 12 milestone making clear he’d wanted it to be part of that release. Next, Jobke explicitly asked developers rullzer and schiessle to review his code. Our Mention Bot then came in and pinged people who had been working on this code which, besides rullzer and schiessle, included nickvergessen.

Next up - this was a quick review turn-around - rullzer and schiessle approved the changes, without giving any comments or feedback beyond an OK and schiessle then merged the code. That was it!

Here is a quick view of the automated test results for another pull request:

The automated tests say 'go'!

The reviewers typically go line by line through the code, sometimes giving tips on how things can be done in a nicer way or pointing out a certain approach worries them. The developer then addresses the issue, either with a fix or an explanation. Here is an example where Icewind has a question, resulting in nickvergessen (who did the pull request) adding a commit that addresses the concern.

Issue fixed

The role of a reviewer often is to ask questions and challenge assumptions. Rather than point out flaws or complain, it is meant to be positive and helpful and generally taken as such. Sometimes, reviewers even simply fix the problem they find, if they have access to the repo, as you see below:

Getting started

So why would reviewing code be a way to get involved in a project? First of all, if you review, you see code all over the place, helping you get an idea of what is what. And second, you get to ask questions! As the saying goes, there are no stupid questions, only stupid answers - so you'll find out how Nextcloud works, while other contributors get to have a 2nd, 3rd or 4th pair of eyes going over their code.

Expectations

So what do we expect from a review? We have our coding guidelines and the review section in our documentation, both recommended reading if you'd like to help doing reviews.

A review isn’t entirely without responsibility, of course - you signify that you can’t find issues. But we can’t all be maintainer of the specific code being reviewed, nor can we always all have enough time and knowledge to do a full, detailed review. So feedback is always welcome, even if you would not consider it a formal review!

Here is LukasReschke expressing a concern, Morris investigating and concluding the current approach is correct.

Discussing worries

Testing made easy

An important element of this is testing. There are very quick and easy ways of setting up a Nextcloud test environment, by just installing PHP, grabbing the Nextcloud source (you can grab a always-up-to-date master-daily from our download servers, look for latest-master.zip) and running the php webserver in the folder where you extracted the source with php -S localhost:5000

You can then proceed to grab the patch that is under testing. That isn’t hard either, as github has a cool feature for that: append .patch at the end of a pull request, say Add support for ratelimiting via annotations by LukasReschke · Pull Request #4336 · nextcloud/server · GitHub and you will get the patch. Try it: https://github.com/nextcloud/server/pull/4336.patch

    1. Download the patch via wget in the Nextcloud server folder: wget https://github.com/nextcloud/server/pull/4336.patch
    2. Apply it, first in a dry-run: patch -p1 --dry-run < 4336.patch and then without the --dry-run
    3. Now simply visit the local host running nextcloud, http://localhost:5000 in our example and see if things work as expected.
You should leave a note, both if you find issues or if you don't so we know you checked the functionality of the patch!

And now you became part of our Review Squad :wink:

Conclusion

Reviewing is arguably one of the most important elements of our collaborative development process. Bringing input in terms of ideas and code from a variety of contributors through a rigorous review process is what makes Nextcloud move so fast yet keep things stable! You can be a part of that process. Just head over to the pulls from any of our sub projects - be it the server itself, or apps like the Calendar, passman or Spreed audio/video calls app. And there are the Android client or even our app store that all have open pull requests ready for your input!
1 Like