On Wed, Feb 12, 2014 at 10:39 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx> 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel