[darcs-users] darcs patch: Resolve issue1584: Provide optimize --up... (and 1 more)

[Eric Kow]

On Sun, Sep 20, 2009 at 16:20:14 +0100, Ganesh Sittampalam wrote:
> This looks pretty good, I have a few comments which could either be  
> address by amend-record or an extra patch as you prefer.

I'll amend (hopefully tonight)

> $ darcs optimize --upgrade
> Checking repository in case of corruption
> The repository is consistent.
> $
>
> Also missing some punctuation there.

Fixed, I think.

> You don't seem to clean out old inventories, though you do clean out old  
> patches. Likewise checkpoints.

Ok, inventories (gz) deleted along with the entire checkpoint
repository.

> It fails on --partial repos without giving a useful message. In theory we 
> could convert those to lazy repos I guess but that seems like extra 
> effort unless there is significant user demand.

This and your "have you tried upgrading a large repo" are why review is
so important to me.  Tests are great; we should have much much more
thorough, more aggressive testing.  A day will come when hlint and
testing become key demands.  But patch review can do special human
things such as asking "Have you considered Foo?".  I guess the trick is
to work out what are all the things you don't need people for (hlint) so
that humans can spend more time thinking of "have you considered Foo"
questions.  There's no particular testing vs review debate going on;
this just random enthusiasm.

On the other hand, I might not get around to fixing this one on my
amend.  I think the best thing to do is to just emit a useful message.

Oh, I never finished my GHC upgrade.  I ran out of disk space!
(I have been running low lately, still need to find things to
delete)

>> +-- imports for optimize --upgrade; to be tidied
>
> Do they actually need tidying? I don't see any warnings.

I had written the imports for this separately from the other imports
so that the patch could cleanly apply to both unstable and 2.3.0.
So the request for tidying was to merge the imports.

>> +upgrade of your repository to the lastest \emph{compatible} format.  
>> Right this
>
> This should read "Right now".

Fixed :-)

> Yes, this is default compression. I think you should remove the '?'.

>> +  -- convert pristine by applying patches
>> +  -- I'm not sure if the best way to convert pristine is to copy it or
>> +  -- to apply the patches.  I believe the apply method is more reliable
>
> I tend to agree since it'll remove any pristine corruption.

Comment made more assertive.

-- 
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/20090922/a59ec91f/attachment-0001.pgp>