Reviewers and maintainers needed for stdlib

Meta
4

I wonder if one reason for the lack of reviewers is that, technically, it’s not entirely straightforward interact with the pull requests?

This was my experience, having occasionally reviewed for stdlib pull requests. Despite having used git to manage my own codes for many years, the review process requires some additional steps that took a little time to figure out (and there are still parts that I don’t understand).

So here’s my step-by-step guide to being a reviewer. It’s incomplete and might be outright wrong in places. But it would be great if we can refine it in this thread, and then make the information clearly available somewhere.

How to review a pull request in stdlib – INCOMPLETE DRAFT

Make sure you have an account on github - if not, go to www.github.com and sign up.

On this page you can browse pull requests that you might be able to review.

There are a couple of ways to review and comment on pull requests:

  1. The easiest is to use the github page for the pull-request to browse the code. You can use the Commits and Files changed tabs to look at the changes. Based on this you can make comments in the Conversation tab. That’s helpful to sort out high-level issues, but won’t show up as a formal “review”.

  2. Typically it is useful to work with the code itself, at least for non-trivial pull requests. See below for how to copy the code to a branch of your local stdlib repository.

  3. I have seen other people suggest changes to pull requests that can be merged via git. I don’t know how to do this, but instructions would be useful, see placeholder below

  4. Finally you can make a review via the github web page. To do this go to the webpage of the pull request, and click on the Files changed tab. If you are logged into github, note in the source-code display you can click on the code and make comments. To do the review you should make a bunch of comments on the code, and then use the Review changes button at the top-right to submit a review along with comments. Then you’re done.

In doing a review it is a good idea to compile the code, run it, and check a few cases. However IMO as a reviewer you shouldn’t feel obliged to write an extensive test suite. Rather, you should confirm that the test-suite provided in the pull request is sufficiently convincing. If it isn’t, by all means suggest new tests. If you have done them and can provide actual code, you can present and discuss it via the Conversation tab.

How to copy the pull-request code into a branch of your local stdlib repository

To do this you first need a local copy of the stdlib git repository. If you don’t already have it, running git clone https://github.com/fortran-lang/stdlib.git in a terminal will copy it into your working directory. If you cd into the stdlib directory and type git-branch, you will see you have the master branch.

To look at the code for the pull request, you can make another branch that contains it. To do this, notice there is a number associated with each pull request, e.g. from the website we see this one https://github.com/fortran-lang/stdlib/pull/272 has number 272. To download this code into a new branch of my local copy of stdlib, I do git fetch origin pull/272/head:BRANCHNAME where BRANCHNAME should be replaced with the name of the new branch I want to make (e.g. I might use pullreq_272 in place of BRANCHNAME). Best if the chosen name does not match any of your existing branches.

After running the git fetch ..... command above, nothing appears to change. However git branch will show that you now have the new branch in addition to the master branch. The command git checkout BRANCHNAME will then take you to that branch (again, replace BRANCHNAME with whatever you named it) . At this point you can look at the source code, compile it, etc. At the end of that, you can get back to the master branch with git checkout master.

How to make changes to the pull request code, and then request that the original author merge them into their repository.

I don’t know.

How to merge changes to your pull request suggested by reviewers with the above.

I don’t know. We had trouble with this in the sorting review.

8 Likes