On Sun, Nov 20, 2005 at 06:32:05PM +0000, Julian Foad wrote:
> >Glad to see this bothered someone else - I've had something similar in my
> >tree for ages. My version removed 'conflict', replacing 'if (conflict)'
> >with 'if (conflict_description != NULL)'.
> >
> >Would anyone object if I checked that in?
>
> It would certainly be nice to replace two variables with one that does the
> same job, but it wouldn't be so obvious for the reader that
> conflict_description is non-null if and only if there is a conflict.
> Perhaps if you renamed the variable to "conflict", that would be clearer?
Oh. Yeah, that's actually in my tree too :-)
> you do this, don't forget to remove the "/* Silence gcc warning */" comment
> which will no longer be appropriate.
Yup.
> I'm neutral. It's the sort of thing that goes best with some other similar
> tidying. It's barely worth a commit on its own, but I wouldn't object to
> it (in principle nor in particular).
>
Yeah, I was thinking the same way, which is why I hadn't bothered.
But now someone's tickled the same area...
> But, hey, there is some related tidying you could do: you might as well
> also move the "conflict_description" variable in svn_wc__merge_prop_diffs()
> into the scope where it is used, which would put it alongside its partner
> "conflict" variable. (And then perhaps eliminate the latter?)
>
Hmm, okay. svn_wc__conflicting_propchanges_p() doesn't actually define
what the value of the description parameter is when it returns FALSE,
but it's in the same source file, so we can presumably use implementation
knowledge.
In fact, that function (and a couple of others in props.h) aren't used
outside of props.c, so they should be static anyway (and removed from
props.h).
Am I missing something?
> Still neutral? Here's a decider question: Are you doing any sort of work
> in that area (debugging/reviewing/studying/coding)?
Yes: I'm writing an email about it :-) But no, not other than that,
though now I've spent some time looking, I might as well fix it up.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 20 21:02:51 2005