Neels Hofmeyr <neels_at_elego.de> writes:
> Hi dev,
>
> The comment for
>
> subversion/include/svn_wc.h (svn_wc_conflicted_p)
>
> is both misleading and incomplete.
>
> Quoting the comment:
> "
> /** Given a @a dir_path under version control, decide if one of its
> * entries (@a entry) is in state of conflict; return the answers in
> * @a text_conflicted_p and @a prop_conflicted_p.
> *
> * (If the entry mentions that a .rej or .prej exist, but they are
> * both removed, assume the conflict has been resolved by the user.)
> */
> "
>
> 1) It does not mention that .rej and .prej refer to file suffixes.
Oddly enough, your patch did not specifically say they refer to file
suffixes either. I tweaked it before committing in r32665.
> 2) It says that a conflict is assumed resolved when *both* mentioned
> conflict files have been removed.
>
> - In fact, this function returns conflict states independently. Each
> conflict is assumed resolved when its respective file can't be found.
>
> - It would be clearer to say "cannot be found" instead of "have been removed".
>
> 3) The comment fails to mention that the entry is not updated, as might be
> expected by the reader.
Agree with all of the above, see r32665.
> There is some discussion on whether this function should actually rather not
> assume anything from missing conflicts files. However, even in its current
> implementation, the comment is insufficient.
>
> I suggest committing this patch, followed by a discussion on the assumptions
> made by the function. (btw, I vaguely favour changing this function to only
> reflect the information in ENTRY, no files involved.)
Feel free to initiate that discussion now.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-23 21:19:28 CEST