2 patches for repository http://darcs.net:

Tue Jun 15 09:44:02 COT 2010  builes.adolfo@googlemail.com
  * Resolve issue 1176: caches interfere with --remote-repo flag

Wed Jun 16 11:23:30 COT 2010  builes.adolfo@googlemail.com
  * Removes duplicated entries after any modification is done to the cache

Attachments

• resolve-issue-1176_-caches-interfere-with-__remote_repo-flag.dpatch
• unnamed

Sorry I forgot --subject here :(.

On Wed, Jun 16, 2010 at 12:20 PM, Adolfo Builes <bugs@darcs.net> wrote:

>
> New submission from Adolfo Builes <builes.adolfo@googlemail.com>:
>
> 2 patches for repository http://darcs.net:
>
> Tue Jun 15 09:44:02 COT 2010  builes.adolfo@googlemail.com
>  * Resolve issue 1176: caches interfere with --remote-repo flag
>
> Wed Jun 16 11:23:30 COT 2010  builes.adolfo@googlemail.com
>  * Removes duplicated entries after any modification is done to the cache
>
> ----------
> files: resolve-issue-1176_-caches-interfere-with-__remote_repo-flag.dpatch,
> unnamed
> messages: 11462
> nosy: abuiles
> status: needs-review
> title: Resolve issue 1176: caches interfere wit... (and 1 more)
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch282>
> __________________________________

Attachments

• unnamed

On Wed, Jun 16, 2010 at 17:20:26 +0000, Adolfo Builes wrote:
> Tue Jun 15 09:44:02 COT 2010  builes.adolfo@googlemail.com
>   * Resolve issue 1176: caches interfere with --remote-repo flag

I already reviewed this, so I'll skip it.

> Wed Jun 16 11:23:30 COT 2010  builes.adolfo@googlemail.com
>   * Removes duplicated entries after any modification is done to the cache

Almost there.  Just one more change (to the haddock)

Removes duplicated entries after any modification is done to the cache
----------------------------------------------------------------------
> +-- | @modifyCache repository@ function@ modifies the cache of
> +--   @repository@ with @function@.  After the modification is done the
> +--   duplicated entries are removed and then the cache is sorted with
> +--   local < http < ssh

There are three problems with the haddock that I can see:

 - You have a syntax error in the top line (note that I never ask people
   to amend haddock syntax errors, A. because I'm so grateful they even
   bother to write them and B. because it's more efficient for me to
   just fix it, point it out and be done with it)

 - You are repeating the documentation for
   'Darcs.Repository.Cache.compareByLocality' when you
   could just as easily link to it.  Duplication is the root of all
   evil.  Imagine we one day decide to change 'compareByLocality' and we
   (inevitably) forget to update this haddock.  It's the kind of thing
   that's subtle, we'll never notice it for years until somebody does
   something wrong...

 - Your comment reveals too much about the implementation of the
   function.  Your job is not to tell me what the code says, but
   I as a user of the function can expect from it.
   
   In particular, X happens, then Y and Z is not entirely appropriate.
   The subtlety here is that (f . sort) is not the same as (sort . f).
   Maybe we should think about this on IRC

Can you fix these?  I think once you address the two key issues above,
I can just apply what you have and maybe do some minor cleaning up


> -  where dr       = DarcsRepository pristine newCache
> -        newCache = ( \ (Ca c) -> Ca $ sortBy compareByLocality c) $ f cache
> +  where dr                = DarcsRepository pristine $ nubAndSort $ f cache
> +        nubAndSort (Ca c) = Ca $ sortBy compareByLocality $ nub c

This seems fine.  I think the way I would written it was something like
the below (again with the Functor idea), but it could just be my taste,
which is flawed :-)
 
  dr = DarcsRepository pristine
     $ cmap (nub . sortBy compareByLocality .  f) cache
  cmap f (Ca c) = Ca (f c)

But you should probably ignore my comment above

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

2 patches for repository http://darcs.net:

Tue Jun 15 09:44:02 COT 2010  builes.adolfo@googlemail.com
  * Resolve issue 1176: caches interfere with --remote-repo flag

Wed Jun 16 17:20:18 COT 2010  builes.adolfo@googlemail.com
  *  Remove duplicated entries after any modification is done to the cache

Attachments

• resolve-issue-1176_-caches-interfere-with-__remote_repo-flag.dpatch
• unnamed

On Wed, Jun 16, 2010 at 22:31:45 +0000, Adolfo Builes wrote:
> Tue Jun 15 09:44:02 COT 2010  builes.adolfo@googlemail.com
>   * Resolve issue 1176: caches interfere with --remote-repo flag
> 
> Wed Jun 16 17:20:18 COT 2010  builes.adolfo@googlemail.com
>   *  Remove duplicated entries after any modification is done to the cache

Applied, thanks!  That's 3 bugs fixed!

 Remove duplicated entries after any modification is done to the cache
----------------------------------------------------------------------
> +-- | @'modifyCache' repository function@ modifies the cache of
> +--   @repository@ with @function@, remove duplicates and sort the results with 'compareByLocality'.

Note that I think the pattern is
  'foo' @arg1 arg2@
The tick marks makes foo a link (although I don't really understand why
we do this for the function itself) and the at sign acts as verbatim.
I've made a minor patch on top of yours to fix this.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Hmm, that's funny.  This one was not tracked by darcswatch.

This patch bundle (with 2 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-5e1308ec71e72cc59ad1f4e4234c323bc4e7eb77

This patch bundle (with 2 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-5e1308ec71e72cc59ad1f4e4234c323bc4e7eb77