Re: Half-baked unused features.

[David Kastrup]

Graham Percival <address@hidden> writes:

> <david>
> we don't *have* a "full review process" in any meaningful sense of the
> term.  Especially not for "cleaning up" things.
>
> As evidence, consider:
> http://codereview.appspot.com/1724041/show
>
> - big initial patch
> - lots of comments about splitting up the patch into smaller,
> easily-understood portions
> - contributor (an unknown person, BTW) does what we ask
> - NOBODY bloody looks at it.  The reworked patch has been rotting away
> for almost 2 months.
>
> That's a huge black mark against our development process.
> </david>

Not the process per se, but try doing this on Rietveld.  Those are lots
of changes in small files.  For every single change, you need to tell
the web interface to show you the file difference.  You look at it, it
looks ok.  Now you need to navigate back to the list of changed files,
remember which file you just looked at, select the next file in the
list, navigate to its change overview.

And so on.  If you just work with the diff posted on the list, you can
read and review the whole kaboodle in one session/bunch.

Rietveld is nice for changes confined to one file.  The more files you
want to review in one patch set, the worse you have to click forth and
back while remembering where you are in sequence.

After your above, quite justified tirade, I went to that changeset with
the intent to review it.  After clicking around for a while, I ran out
of motivation.

This is the sort of change you want to review and comment on in one
piece.  Linearly.  Without being forced to do hundreds of mouse clicks.
The scroll wheel should be all you need unless you want to comment.

-- 
David Kastrup