Ganesh Sittampalam <[email protected]> added the comment: This looks good. I think the new feature has been discussed enough in the associated bug that it's ok to add without further discussion.
Some comments justifying the changes to myself and also some style points below. OK, so finalizeRepositoryChanges is called by AmendRecord, Apply, Get, Pull, Tag, Unrecord, Unrevert, Rollback, Record, Optimize. So the things you haven't updated are Unrecord, Unrevert, Tag, Optimize and Get. I agree that makes sense. Minor style points: - it would have been better to separate the refactoring and the new functionality into separate patches, to make it easier to later understand what changed and undo things if necessary. - could the similar code in Record and AmendRecord for asking whether to go ahead be factored out? I'm happy to accept this as is, but I'll wait a bit in case you want to address the above points with new patch(es). ---------- status: needs-screening -> review-in-progress __________________________________ Darcs bug tracker <[email protected]> <http://bugs.darcs.net/patch454> __________________________________ _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
