{"id":12811,"date":"2017-04-14T10:03:52","date_gmt":"2017-04-14T10:03:52","guid":{"rendered":"https:\/\/nextcloud.com\/?p=2161"},"modified":"2017-04-14T10:03:52","modified_gmt":"2017-04-14T10:03:52","slug":"get-involved-in-nextcloud-by-reviewing-pulls","status":"publish","type":"post","link":"https:\/\/nextcloud.com\/fr\/blog\/get-involved-in-nextcloud-by-reviewing-pulls\/","title":{"rendered":"Get involved in Nextcloud by reviewing pulls!"},"content":{"rendered":"

Starting to contribute to Nextcloud isn’t entirely trivial. Openhub calculates<\/a> 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!<\/p>\n

What is reviewing?<\/h2>\n

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:<\/p>\n

\"a
\nWhat you see is, first, that pull request number 4316<\/a> 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.<\/p>\n

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.<\/p>\n

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!<\/p>\n

Here is a quick view of the automated test results for another pull request:<\/p>\n

\"The<\/p>\n

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.<\/p>\n

\"Issue<\/p>\n

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:<\/p>\n

\"\"<\/p>\n

Getting started<\/h2>\n

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.<\/p>\n

Expectations<\/h3>\n

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

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!<\/p>\n

Here is LukasReschke expressing a concern, Morris investigating and concluding the current approach is correct.<\/p>\n

\"Discussing<\/p>\n

Testing made easy<\/h3>\n

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

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<\/code> at the end of a pull request, say https:\/\/github.com\/nextcloud\/server\/pull\/4336<\/code> and you will get the patch. Try it: https:\/\/github.com\/nextcloud\/server\/pull\/4336.patch<\/code><\/a><\/p>\n