1 patch for repository http://darcs.net:

Tue Aug 10 08:08:09 COT 2010  builes.adolfo@googlemail.com
  * Adding some haddock to Cache.hs

Attachments

• adding-some-haddock-to-cache_hs.dpatch
• unnamed

Adding some haddock to Cache.hs
-------------------------------
I'm always very grateful for haddocks!  One day we'll have a proper
Darcs API, stable, clean, sensible and appropriately documented, and
you have just contributed to the cause.

That said, I have suggestions...

> +-- | @fetchFileUsingCache@ receives the hash of the file, the repository
> +-- in which is going to be located, and the directory for which the
> +-- hashed file belongs.

I'm a bit confused by this, because the type signature seems to be
pointing at less stuff.

Also perhaps you could make this a little bit more terse by using the
annotation on type signature style?

fetchFileUsingCache :: Cache
                    -> HashedDir -- ^ target repo
                    -> String    -- ^ hash
                    -> IO (String, B.ByteString)

(I'm not sure what the best practice is, just exploring, eh?)

>     It uses @fetchFileUsingCachePrivate@ to fetch
> +-- the file

No, that's an implementation detail that I don't care about if
I'm using the Darcs API.

> , it takes as source the first entry of the cache, it the
> +-- file is not in that source, it tries to get for the next one. It
> +-- will transverse the cache list until it is empty or it fetches the
> +-- file, if the file can't be fetch the operation fails.

Ah, that's a tricky one to word because you want to convey three
ideas: trying the sources in order, returning on first success
and failing if you exhaust the sources.

Hmm, how about something like this?

  It tries to fetch the file from one of the sources, trying
  them in order one by one.  If the file cannot be fetched
  from any of the sources, this operation fails.

Phew! This is hard! I hope you don't mind my constant rewording.
Submitting the haddock patch is a very important first step because
it calls our attention to what needs expressing.

> +-- | copyFileUsingCache, copy or hardlinks a file to the cache, usually the global cache, if
> +--   the cache directory doesn't exist the function will fail.
>  copyFileUsingCache :: OrOnlySpeculate -> Cache -> HashedDir -> String -> IO ()
>  copyFileUsingCache oos (Ca cache) subdir f =
>      do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir subdir)++"/"++f

First, I think using a convention like

@copyFileUsingCache oos ca dir f@ copies the file @f@ to the cache @ca@

makes the haddock a bit clearer because it shows you which arguments
refer to which high-level concept (this matters most when you have 
arguments of the same type, which is not the case here)

Second, you should split into two sentences, before "if the cache
directory".

Thanks!

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.

1 patch for repository http://darcs.net:

Tue Aug 10 16:51:16 COT 2010  builes.adolfo@googlemail.com
  * Adding some haddock to Cache.hs

Attachments

• adding-some-haddock-to-cache_hs.dpatch
• unnamed

Looks good.  Short and to the point, explains the things a user of these
functions needs to know.  Thanks!

Adding some haddock to Cache.hs
-------------------------------
> builes.adolfo@googlemail.com**20100810215116
>  Ignore-this: 34560d0bfcf1da1690742421b835caf
> ] hunk ./src/Darcs/Repository/Cache.hs 155
>                | length h == 75 = B.length s == read (take 10 h) && sha256sum s == drop 11 h
>                | otherwise = False
>  
> -
> +-- |@fetchFileUsingCache cache dir hash@ receives a list of caches
> +--  @cache@, the directory for which that file belongs @dir@ and the
> +--  @hash@ of the file to fetch.  It tries to fetch the file from one
> +--  of the sources, trying them in order one by one.  If the file
> +--  cannot be fetched from any of the sources, this operation fails.
>  fetchFileUsingCache :: Cache -> HashedDir -> String -> IO (String, B.ByteString)
>  fetchFileUsingCache = fetchFileUsingCachePrivate Anywhere
>  
> hunk ./src/Darcs/Repository/Cache.hs 209
>  
>  data OrOnlySpeculate = ActuallyCopy | OnlySpeculate deriving ( Eq )
>  
> +-- | @copyFileUsingCache oos cache subdir f@ copy or hardlinks the
> +--   file @f@ to one of the caches in @cache@,usually the global cache.
> +--   If a writable cache doesn't exist the function will fail.
>  copyFileUsingCache :: OrOnlySpeculate -> Cache -> HashedDir -> String -> IO ()
>  copyFileUsingCache oos (Ca cache) subdir f =
>      do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir subdir)++"/"++f
> 

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-f817c6d5be40d19aa75fc8c5bc11fbaa50580d95

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-f817c6d5be40d19aa75fc8c5bc11fbaa50580d95