That's very interesting; thanks for the link.
One part that jumps out at me:
: ... many teams that review code don't have a good way of tracking
defects found
: during review and ensuring that bugs are actually fixed before the
review is complete
We don't have this, and with github reviews it's
I completely agree with Ian's point about code needing to be
self-explanatory and stand on it's own.
That said, the article mentions that the process of creating review
annotations encourages the author to review their own work in a way that
they may have not done otherwise, eliminating problems b
On 2014-06-25 09:43, roger peppe wrote:
About pre-review annotations, I agree with Ian that the code should be
documented
well enough that someone coming to it from scratch can understand it, but
I also wonder if there is a room for review-specific comments, talking about
reasons for the changes
I agree that the code needs to be self-explanatory enough to not require
annotations, but annotations can be useful - especially for larger changes.
Suggesting the order for code to be reviewed is certainly useful if you're
reviewing code in a part of the system you aren't familiar with
On Wed, J
We've tried to encourage what we call 'reviewer comments' that are intended
to be to help the reviewer follow the code. There are definitely two chunks
of things that tend to get written. It's quite often to find a reviewer
comment that the reviewer then suggests is put into the code itself.
Howev
Agreed, but for a slightly different reason. The suggestion is to
annotate the patch with the reason for the change, rather than the
code itself, which might indeed lead to a different kind of comment.
While this might be useful, one of the interesting outcomes of code
reviewing is that it forces t
Thanks, John. Several nice ideas there. I especially like the data
backing the first few points.. it provides evidence to something we
intuitively understand.
I also wrote some points about this same topic, but from a slightly
different perspective, last year:
http://blog.labix.org/2013/02/06
Today as on call reviewer I was looking through some pull requests
specific to actions, and it brought up many questions.
On reading the spec[1] and looking at the state documents as they are in
trunk now, I was quickly coming to the conclusion that the documents
need to change.
Right now we have
This looks great, with one or two remarks below.
On 26 June 2014 05:47, Tim Penhey wrote:
> On reading the spec[1] and looking at the state documents as they are in
> trunk now, I was quickly coming to the conclusion that the documents
> need to change.
>
> I think these should be combined, and t
