Bug 38388

Another approach that can be taken in conjunction with this approach is to move/refactor methods defined in both files to a third location. The third location can be VCSUtils.pm or a new library file in webkitperl/ named something like svnApplyUtils.pm. I started doing this myself here, for instance: http://trac.webkit.org/changeset/52692 Question: are you suggesting that, in the end, svn-unapply should be invoked by calling svn-apply (e.g. by passing a -R/--reverse flag)? That sounds good. If so, svn-unapply could be a wrapper for svn-apply until the callers are modified.

(In reply to comment #1) > Another approach that can be taken in conjunction with this approach is to > move/refactor methods defined in both files to a third location. The third > location can be VCSUtils.pm or a new library file in webkitperl/ named > something like svnApplyUtils.pm. I started doing this myself here, for > instance: > > http://trac.webkit.org/changeset/52692 If we follow this direction the unit-testing can be more precise not by testing the whole file(s), but subroutines directly. If the merged functions can be used in other files I like the idea to put it into VCSUtils, if not, separated file would be better. > Question: are you suggesting that, in the end, svn-unapply should be invoked by > calling svn-apply (e.g. by passing a -R/--reverse flag)? That sounds good. If > so, svn-unapply could be a wrapper for svn-apply until the callers are > modified. Yes. I was thinking on the same.

(In reply to comment #2) > (In reply to comment #1) > > Another approach that can be taken in conjunction with this approach is to > > move/refactor methods defined in both files to a third location. > > If we follow this direction the unit-testing can be more precise not by testing > the whole file(s), but subroutines directly. We can still do this either way. For example, if you look at VCSUtils_unittest/, you will see that it contains unit tests for individual subroutines defined in VCSUtils.pm. In the end, perhaps svn-apply can have a top-level "main()" method. Then if one wants to go the route of testing the whole file, testing that single subroutine is almost as good from a code coverage standpoint as testing the "whole file". It won't be as necessary to test the script by invoking it as a script.

(In reply to comment #1) > Question: are you suggesting that, in the end, svn-unapply should be invoked by > calling svn-apply (e.g. by passing a -R/--reverse flag)? That sounds good. If > so, svn-unapply could be a wrapper for svn-apply until the callers are > modified. Please don't remove the ability to call svn-apply and svn-unapply. I have been happily using these scripts for many years and really don't want to change my habits. Merging the implementations sounds fine.

Retitling with "svn-apply" at the beginning of the title.