Replace the 'unpack' command with a more general 'get'. #1075

Merged
merged 4 commits into from Jan 11, 2013

Conversation

Projects
None yet
3 participants
Member

23Skidoo commented Oct 20, 2012

This is @jmillikin's cabal fork implementation (#2) with the UI changes requested by @dcoutts.

The first patch contains John's original implementation of cabal fork. The only differences are cosmetic: UI commands fork and unpack are merged (this is the new cabal get command), and the code for forking and unpacking is moved to a single module (Distribution.Client.Get).

The second patch removes duplication between the functions fork and unpack.

The third patch adds support for the repo kind argument to --source-repository (cabal get --source-repository=this PACKAGENAME now works).

Member

dcoutts commented Oct 29, 2012

Ah hah, great. Will have a close look.

Member

dcoutts commented Oct 30, 2012

Ok, it looks good. One further thing that I think we need: some degree of backwards compat for the 'unpack' ui. The unpack command has been around for ages and people are familiar with it. We shouldn't have it simply disappear instantly.

So one option is to make unpack just an alias for get, or alternatively keep unpack as a separate thing command, that just calls the same code. We can decide later if we want to remove unpack from the list of commands listed in --help.

Member

23Skidoo commented Oct 30, 2012

@dcoutts Fixed in dac5d47.

Member

dcoutts commented Nov 1, 2012

Cool. I'll apply that. Though I might drop the deprecation warning part, it's pretty cheap for us to keep it as a hidden alias indefinitely.

Member

23Skidoo commented Nov 1, 2012

Yes, we can keep it indefinitely, but I think that nudging people to use get is not a bad idea. I don't care much, though.

Member

23Skidoo commented Nov 3, 2012

Is there something else I can do to get this merged in?

Owner

tibbe commented Nov 3, 2012

@23Skidoo I left it up to @dcoutts to review/apply.

Member

23Skidoo commented Nov 20, 2012

@dcoutts Can I merge this in? You can always remove the deprecation warning later if you want.

Member

dcoutts commented Dec 10, 2012

Ok, I've got through many of the other patches / pull reqs, I guess this one is next for review. We ought to be able to get it into this next release...

Member

23Skidoo commented Jan 1, 2013

@dcoutts - do you have any objections if I merge this in?

23Skidoo added some commits Oct 19, 2012

@23Skidoo 23Skidoo Replace the 'unpack' command with a more general 'get'.
'cabal get PACKAGE' is the new name of 'cabal unpack'.

'cabal get --source-repository' reads the source-repositories from a package's
description, determines which VCS to use, and then creates a local repository or
branch of the package's repository.

'cabal get --source-repository=[head|this|...]' additionally allows to specify
which source-repository to use.

Based on the code originally written by John Millikin <jmillikin@gmail.com>.
f8665a4
@23Skidoo 23Skidoo Remove duplication between 'unpack' and 'fork'. 0e2d03e
@23Skidoo 23Skidoo Support the repo kind argument to '--source-repository'. 0a1e67e
@23Skidoo 23Skidoo Make 'unpack' a (hidden, deprecated) alias for 'get'. 7888be1
Member

23Skidoo commented Jan 11, 2013

I'm merging this in. The only slightly controversial issue here is the deprecation warning for cabal unpack, and it can be reverted later.

@23Skidoo 23Skidoo added a commit that referenced this pull request Jan 11, 2013

@23Skidoo 23Skidoo Merge pull request #1075 from 23Skidoo/cabal-get
Replace the 'unpack' command with a more general 'get'.
7d73e69

@23Skidoo 23Skidoo merged commit 7d73e69 into haskell:master Jan 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment