Malcolm Rowe wrote:
>
>>- svn_string_t *conflict_description;
>>+ svn_string_t *conflict_description = NULL; /* Silence gcc warning */
>> svn_boolean_t is_normal, conflict = FALSE;
>
> 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?
Trivial subject --> long reply :-)
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? Or just put a comment "There is
a conflict" at the point of the "if". If you do this, don't forget to remove
the "/* Silence gcc warning */" comment which will no longer be appropriate.
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).
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?)
Still neutral? Here's a decider question: Are you doing any sort of work in
that area (debugging/reviewing/studying/coding)? If so, given that this
bothers you (even slightly), certainly go ahead.
FWIW, I keep a "misc.patch" in my Subversion source tree, holding little
patches that aren't quite useful enough to check in or post to the mailing
list, but might be worth doing one day if an opportunity arises from doing
related changes or if the subject crops up.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 20 19:32:56 2005