<div dir="ltr">On Sat, Feb 9, 2013 at 12:42 AM, Brodie Rao <<a href="mailto:brodie@sf.io">brodie@sf.io</a>> wrote:<br>><br>> On Fri, Feb 8, 2013 at 10:23 PM, Matt Mackall <<a href="mailto:mpm@selenic.com">mpm@selenic.com</a>> wrote:<br>
> > On Fri, 2013-02-08 at 21:45 +0000, Brodie Rao wrote:<br>> >> On Fri, Feb 8, 2013 at 9:31 PM, Idan Kamara <<a href="mailto:idankk86@gmail.com">idankk86@gmail.com</a>> wrote:<br>> >> > On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <<a href="mailto:brodie@sf.io">brodie@sf.io</a>> wrote:<br>
> >> >><br>> >> >> # HG changeset patch<br>> >> >> # User Brodie Rao <<a href="mailto:brodie@sf.io">brodie@sf.io</a>><br>> >> >> # Date 1360357714 0<br>> >> >> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf<br>
> >> >> # Parent  2fefd1170bf269e26bb304553009f38e0117c342<br>> >> >> amend: support amending merge changesets (issue3778)<br>> >> ><br>> >> > I haven't looked in depth at the tests you added (seems<br>
> >> > like you're not testing conflicts?) but I'm surprised it works<br>> >> > with so little and trivial code added.<br>> >><br>> >> I don't think a merge with content-level conflicts would be that<br>
> >> different than what's tested now. A merge with manifest-level<br>> >> conflicts might be interesting though.<br>> >><br>> >> > I recall Matt saying this is going to be quite tricky to get<br>
> >> > right (I also believe I tried to do something close<br>> >> > to what you did here and saw it misbehave sometimes,<br>> >> > but I can't remember where exactly right now).<br>
> >><br>> >> If you don't handle the fact that ctx.files() can be totally wrong in<br>> >> a lot of merges (files that were changed aren't in the list, files<br>> >> that were merged but didn't have content-level changes are in the<br>
> >> list, etc), I can see why you'd run into weird issues.<br>> >><br>> >> But, accounting for that, I can't think of why this should be<br>> >> difficult to implement, but perhaps I'm overlooking something.<br>
> ><br>> > I think the particular issues I had in mind were to do with the nasty<br>> > business in localrepo._filecommit. We have to figure out which<br>> > file-level parent revisions to use for merged files and we can get<br>
> > confused with merges involving copies especially.<br>><br>> As mentioned in person, I don't think this is an issue. Amend properly<br>> preserves copy information, and it just reuses the filelogs from the<br>
> temporary commit, which are carried over from the merge commit we're<br>> replacing, so all the filelogs should be consistent.<br>><br>> In other words, because amend properly preserves everything, it<br>
> doesn't need to be aware of the things localrepo._filecommit does.<div><br></div><div>Ok, wouldn't hurt to add tests for conflict resolution then.<br></div></div>