Hi, On Thu, Apr 23, 2009 at 20:48:39 +0400, Dmitry Kurochkin wrote: > Patches look good. The first patch makes code much cleaner and > simpler. The second one is tricky but does the right thing.
Thanks for the review! > I think they should be applied if Reinier tested them. Having seen (at a distance) Reinier do lots of testing at the sprint, I'm going to go ahead and apply these now, but it sounds like we're in for a few tweaks? > Few comments: > > FileContent fc_maxline is updated only in delete_line. I understand > that this is sufficient for code to work. But FileContent comment and > field name suggest that it should be updated in insert_line as well. A > bit misleading. > > hunk ./src/Darcs/Patch/Check.hs 31 > > import Data.List (isPrefixOf) > > import Control.Monad.State ( State, evalState, runState ) > > import Control.Monad.State.Class ( get, put, modify ) > > +-- use Map, not IntMap, because Map has mapKeys and IntMap hasn't > > +import Data.Map ( Map ) > > +import qualified Data.Map as M ( mapKeys, delete, insert, empty, lookup, > > null ) > > > > Would this comment be more useful near the use of Map, > i.e. FileContents definition? > > hunk ./src/Darcs/Patch/Test.hs 49 > > import Control.Monad ( liftM, liftM2, liftM3, liftM4, replicateM ) > > > > import Darcs.Patch.Info ( PatchInfo, patchinfo ) > > -import Darcs.Patch.Check ( PatchCheck, Possibly(..), > > +import Darcs.Patch.Check ( PatchCheck, > > check_move, remove_dir, create_dir, > > is_valid, insert_line, file_empty, file_exists, > > delete_line, modify_file, create_file, remove_file, > hunk ./src/Darcs/Patch/Test.hs 53 > > - do_check, do_verbose_check, > > + do_check, do_verbose_check, FileContents(..) > > ) > > import RegChars ( regChars ) > > import ByteStringUtils ( linesPS ) > > This could be a nice one line change :) -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
