I’ve recently contributed a review of the awesome Ropenaq package by Maelle Salmon (here’s a link to the review issue). The other reviewer was @andy_teucher . I had great time & learned a lot. Definitely the best review (of paper/code/anything) experience of my career so far!
That said, I was thinking we could have a discussion of how to make this review process even more awesome. Its already a positive experience for reviewers and (i think?) for package authors. Here’s a few points that I thought we could discuss:
How to reference specific lines of code. I found myself frequently writing things like “So at the start of this function” or “in file X at line Y”. I know I could have linked to the specific line in via github, but I saw two problems with that. First, it is kind of laborious (if you have lots of such comments), and second, one can’t look at code and tell what the comments are (you can only go from the comments to the code that the reviewer indicated). I’m also not sure what the best practice is here. Link to a specific commit? will that get confusing? What helps the package author the most?
interactions between reviewers: I was fortunate to review alongside @andy_teucher who did a very thorough job. Andy and I have similar (but not identical) skills and opinions about R packages. As it turned out, in some places we had nearly identical comments and in others, we covered different points. Where we were different, it probably would have been better to coordinate (ie Andy does the mapping part) and in others, it might have been interesting to compare notes (Andy and I both wrote nearly identical code as a suggestion)
suggesting code changes I didn’t see anything in the reviewer guide about what the best approach here is. I settled on just dropping in code suggestions as formatted code blocks. But that quickly gets out of hand if there are lots of suggestions. PRs feel like a more natural way to make those suggestions, but that would fragment the review.
carrying on the conversation. The discussion between Andy, Maelle and I is big. Andy and I each wrote about 1500 words in our reviews, and Maelle’s response to us is about the same length! This is awesome and very interesting. But it is also getting to the point of “regarding your fourth point in the third section”. There’s also things that Andy said that i want to respond to, and responses of Maelle’s to Andy that I’d comment on…
I was wondering if there is some github workflow that might make this process easier. For example, what if the entire package was submitted as a PR to an empty repository at ropensci? Say, as a pull request between an empty branch and a branch containing the entire source of the package. I can see some pros and cons to that approach
- reviewers could make inline comments on the diff.
- Separate issues could be opened to cover larger points.
- more economical: When one reviewer sees that the other has commented on something already, they can decide to either move on to a different part of the code, or respond to the first reviewer’s comments.
- nice for future users, who could view this discussion where the package lives (ie as a closed issue at
- if one reviewer is fast / thorough, the other might be reduced to just saying “+1” everywhere, which would be unfair
- obliging authors to learn branching
- cluttering ropensci’s gallery of repos with all these in-progress things
Those are some thoughts from me! What do you guys think?