Re: [egit-dev] Using Gerrit to see diffs between changesets

["Shawn O. Pearce" <spearce@xxxxxxxxxxx>]

Alex Blewitt <alex.blewitt@xxxxxxxxx> wrote:
> I spent a while reviewing (and coming up with comments for) this  
> changeset:
>
> http://egit.eclipse.org/r/#change,41
>
> However, whilst I was doing that, another one appeared. Is there any way 
> to see what (a) files I've seen already,

Not easily in the new patch set, no.  The little green check means
you've looked at this revision of this file, but doesn't help you
much between patch sets when a file hasn't changed.

> and (b) diffs since the last 
> time I saw the changes?

Open the first file of the new patch set, expand the History foldy
at the top, click the prior patch set in the left hand column.

> Also, I've seen a few red blobs shown in Gerrit - what are they, and  
> should I be commenting on them? I'm guessing that they might be some  
> kind of trailing space.

Yes, these are whitespace errors.  They usually crop up on Javadoc
where the Eclipse code formatter is adding a space after after the *
on an otherwise empty line.

> Lastly, it would be good to know whether these level of reviews are  
> useful,

Its useful, if somewhat annoying.  I hate having people nitpick
my code, but I think all of your comments were constructive and
produced a better result, so after I got over being annoyed I did
the requested cleanups, and am happy that I did so.

> I think these could 
> easily be applied after the branch has been committed to master (and then 
> subsequent changesets show them from there) which would at least make my 
> life at reviewing them slightly easier ...

Robin and I have a somewhat conservative policy when applying code,
we want to apply as perfect of a change as we can reasonably get.
If there are problems known up front with something, we want to
fix those before we apply it.

So, in the past we've leaned more towards cleaning up issues before
application, not after.

-- 
Shawn.