[darcs-users] [RESEND] Rename hashSlurped, slurpHashed and sync... (and 2 more)

Eric Y. Kow eric.kow at gmail.com
Tue Sep 2 10:18:41 UTC 2008


Just reviewing.  

> Tue Sep  2 02:44:02 CEST 2008  me at mornfall.net
>   * Rename hashSlurped, slurpHashed and syncHashed to writeHashedPristine, slurpHashedPristine and syncHashedPristine, respectively.

Just a straightforward rename.  In fact, since David's already oked this
in principle, I would just fast-track.
 
> Tue Sep  2 03:04:05 CEST 2008  me at mornfall.net
>   * Change type of subdir parameter in Cache/HashedIO functions from String to HashedDir.

I like the idea behind this refactor.  It also falls into a general
trend of using explicit types rather than bools or strings to make
things explicit and more resistant to future-us making silly mistakes.
   
> Tue Sep  2 04:05:32 CEST 2008  me at mornfall.net
>   * Haddock the {slurp,write,sync}HashedPristine functions in HashedIO.

Another fast-trackee

Content-Description: A darcs patch for your repository!

> [Rename hashSlurped, slurpHashed and syncHashed to writeHashedPristine, slurpHashedPristine and syncHashedPristine, respectively.
> me at mornfall.net**20080902004402] hunk ./src/Darcs/Repository/HashedIO.lhs 326

>  syncHashed :: Cache -> Slurpy -> String -> IO ()
>  syncHashed c s r = do runStateT sh $ HashDir {permissions=RW, cache=c, options=[], rootHash=r, hashDir="pristine.hashed" }
> -                      return ()
> +                              return ()

:-) darcs replace won't fix indentation for you

> replace ./src/Darcs/Repository/HashedIO.lhs [A-Za-z_0-9] hashSlurped writeHashedPristine
> replace ./src/Darcs/Repository/HashedIO.lhs [A-Za-z_0-9] slurpHashed slurpHashedPristine
> replace ./src/Darcs/Repository/HashedIO.lhs [A-Za-z_0-9] syncHashed syncHashedPristine
> replace ./src/Darcs/Repository/HashedRepo.lhs [A-Za-z_0-9] hashSlurped writeHashedPristine
> replace ./src/Darcs/Repository/HashedRepo.lhs [A-Za-z_0-9] slurpHashed slurpHashedPristine
> replace ./src/Darcs/Repository/HashedRepo.lhs [A-Za-z_0-9] syncHashed syncHashedPristine

Looks good.

> +data HashedDir = HashedPristineDir | HashedPatchesDir | HashedInventoriesDir

There is a potential confusion between HashedDir and HashDir.

But looking at the rest of the code, I see that this HashDir data type
is only used for interacting with pristine (and not, for example, with
the patch dir), so maybe the latter should be renamed to reflect this.
For that matter, it seems like there's a lot of HashedIO functions that
will also want renaming if we're going to de-parameterise them.

> -findFileMtimeUsingCache :: Cache -> String -> String -> IO EpochTime
> +findFileMtimeUsingCache :: Cache -> HashedDir -> String -> IO EpochTime
>  findFileMtimeUsingCache (Ca cache) subdir f = mt cache
>      where mt [] = return undefined_time
>            mt (Cache Repo Writable r:_) = (modificationTime `fmap`
> hunk ./src/Darcs/Repository/Cache.lhs 97
> -                                          getSymbolicLinkStatus (r++"/"++darcsdir++"/"++subdir++"/"++f))
> +                                          getSymbolicLinkStatus (r++"/"++darcsdir++"/"++(hashedDir subdir)++"/"++f))

Silly style detail: you don't need the parens around (hashedDir subdir)
because in Haskell, function application has higher precedence than any
infix operator

> +gethashmtime h = do HashDir _ c _ _ <- get
> +                    lift $ unsafeInterleaveIO $ findFileMtimeUsingCache c HashedPristineDir h

Here's an example of the kind of function which probably should be
renamed.  getPristineHashMtime?
  
> [Haddock the {slurp,write,sync}HashedPristine functions in HashedIO.
> me at mornfall.net**20080902020532] hunk ./src/Darcs/Repository/HashedIO.lhs 264
>                        opts <- gets options
>                        lift $ writeFileUsingCache c opts HashedPristineDir ps

Hooray!  I like that you've written these in a high level way that lets
me know these functions are for.
  
> +-- |Write contents of a Slurpy into hashed pristine. Only files that have not
> +-- not yet been hashed (that is, the hash corresponding to their content is
> +-- already present in hashed pristine) will be written out, so it is efficient
> +-- to use this function to update existing pristine cache. Note that the
> +-- pristine root hash will *not* be updated. You need to do that manually.

Do have a look at
  http://www.haskell.org/haddock/doc/html/index.html
for markup hints

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20080902/2fa13c4b/attachment-0001.pgp 


More information about the darcs-users mailing list