On Fri, Aug 9, 2013 at 5:18 PM, Daniel Shahaf <danie...@elego.de> wrote:
> Fredrik Orderud wrote on Tue, Aug 06, 2013 at 22:40:02 +0200: > > I've now written an XFAIL test for merging the same change change twice. > > Patch attached. The test fails as expected due to the lack of conflict. > > This is the first test I've ever written for subversion, so there are > > probably some improvement opportunities. I suspect that one weak spot is > > that a "greek-tree" structure is generated without being used in the > test. > > Also, comparison of console output for merge results feels a little > fragile. > > Yes, agreed on both points. You could use A/mu instead of creating a new > file, and use one of the svntest/actions.py helpers that parse the > output of merge/status instead of depending on the exact byte-by-byte > expected output. More below. > > > Please let me know if there are any comments to the patch, and I'll do my > > best to improve it. Otherwise, it would be great it the test could be > > integrated, so that issue #4405 can receive some test coverage. > > Please use text/plain MIME type. This makes review easier. Usually > *.txt extesion achieves this. > > I think the patch is correct *if* we agree that "Merge the same change > twice" should raise a conflict. Prior discussion in this thread > indicates that in present svn that scenario is explicitly decided not to > be a conflict. Therefore, I am not going to commit this patch. > > I think the best thing to do is to attach it to the issue tracker on the > issue tracking Julian's suggestion of a "strict conflicts" mode. That > way, we can apply the patch once we start implementing the "strict" > mode. (We tend not to change our svntest/*.py interfaces that much, so > I wouldn't be concerned about bitrot.) > > So, in summary: thanks for the patch, I think it's correct, but I'm not > going to apply it for the reasons above. > > Does that make sense? > Thank you for the response Daniel. I agree with your reasoning and will be satisfied with a "strict conflicts" mode in subversion. That's fine for me. :-) I will attempt to improve the patch based on Johan's and your feedback, and attach it to the issue-tracker sometime the next few days. Best regards, Fredrik
