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:
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 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.
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:
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.
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.
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 https://github.com/nextcloud/server/pull/4336 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.patch2. Apply it, first in a dry-run: patch -p1 --dry-run < 4336.patch and then without the --dry-run3. 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 😉
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!
At Nextcloud, we are dedicated to sharing our mission, values and products with you. That’s why this year, we’re headed to the biggest IT, digital technology and open source events all over Europe! If you have the opportunity to attend an event, you’ll: We hope to see you this month at the following events! ESUP […]
EsLibre Conference The EsLibre conference is a free software conference centered around libre technologies and open source projects. This year, Nextcloud will give a talk and workshop: “nc-env: a tool to mount Nextcloud environments on your computer!” Senior Sales Engineer, Pietro Marini will lead the workshop from 12-2pm, and there are only 15 spots available, […]
Nextcloud Enterprise Day is soon here and we have a growing list of speakers providing insightful talks! One of our speakers is Nextcloud customer Samaritan’s Purse – a nondenominational evangelical Christian organization based in North Carolina, U.S.A. In preparation for their talk at Nextcloud Enterprise Day on June 13th, we asked them a few questions! […]