38388 2010-04-30 06:22:19 -0700 svn-apply: merge its implementation with svn-unapply 2010-05-03 15:48:23 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC All NEW P2 Normal --- 1 zoltan webkit-unassigned cjerdonek dbates sam oldest_to_newest 219462 0 zoltan 2010-04-30 06:22:19 -0700 Merge svn-apply and svn-unapply into one file, since these define nearly the same methods. 219503 1 cjerdonek 2010-04-30 09:21:04 -0700 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. 219505 2 zoltan 2010-04-30 09:33:01 -0700 (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. 219508 3 cjerdonek 2010-04-30 09:45:11 -0700 (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. 219736 4 sam 2010-05-01 11:55:41 -0700 (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. 220339 5 cjerdonek 2010-05-03 15:46:53 -0700 Retitling with "svn-apply" at the beginning of the title.