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

[Marc-André Lureau]

On Wed, Feb 12, 2014 at 10:39 AM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> 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?"

ok

> 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?).

In general, I agree, but I consider this to be simple, non-essential
and hopefully replacable in a near future with a real GUri parser (or
equivalent, like SoupUri)

> 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\

I'll split this change off then.



-- 
Marc-André Lureau