Summary: | svn-apply: merge its implementation with svn-unapply | ||
---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Horvath <zoltan> |
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW --- | ||
Severity: | Normal | CC: | cjerdonek, dbates, sam |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | All |
Description
Zoltan Horvath
2010-04-30 06:22:19 PDT
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. |