[Spice-devel] [spice-gtk 03/13] spice-proxy: parse user and pass

[Christophe Fergeau]

On Tue, Feb 11, 2014 at 06:15:15PM +0100, Marc-André Lureau wrote:
> I am not fond of super-extra-small changes, I tend to make things that
> belong together in the same patch. This one could be splitted in 5
> patches or more, I don't see the point. All I want is a reasonable set
> of related changes that don't break things.

Well, this makes review more complicated. I was focused on reviewing a
change parsing user and password, so I tried to interpret this change as
something helping with that. When it did not seem useful for parsing
username and password, I was still stuck with "am I missing something, or
is it just a parsing improvement?"
If this small change were to be buggy in some corner cases a few years from
now, we'd be misled again if git bisect points us at a commit adding
user/password parsing rather than at a small commit doing some minor
cleanup. And we'll also won't know anything about the intent of this change
(was it a small and obvious cleanup? was it required for some reason? what
was that reason?).
So while it may make things more convenient now for the person doing the
commit, it's in my opinion (well not just me) better to avoid having silent
changes in commits.

> So should I split it or you agree it's fine as part of parsing changes?

I initially was planning to tell you to just go with it, and now I just
convinced myself that it's better to split it /o\

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140212/c9f9e55b/attachment.pgp>