Add support for alternative SVN structures #1037

Merged
merged 3 commits into from Oct 19, 2012

Conversation

Projects
None yet
5 participants
@bboer
Contributor

bboer commented Aug 29, 2012

This feature adds basic support for the SvnDriver to work with a non-default directory structure.

By default most repositories are set-up with the trunk/branches/tags directories, however since Subversion does not enforce this (best-practice) structure it does happen that within companies the folder structure (slightly) differs from the best practice.

I added this pull request because (surprising) the company I work for uses a different structure which is a company policy to follow, so to be able to use composer (since I like it a lot) this feature could really make my life a lot better (and maybe others as well if they use a different structure).

Regards, Bart

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 29, 2012

This pull request passes (merged 93628c4 into e2f8098).

This pull request passes (merged 93628c4 into e2f8098).

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Aug 29, 2012

Member

Could you just change trunkLocation and others to trunk-path (or at least trunk-location)? I'm not sure if location or path is best, but we use dash separated words everywhere in composer.json, so it'd be best to follow that.

Member

Seldaek commented Aug 29, 2012

Could you just change trunkLocation and others to trunk-path (or at least trunk-location)? I'm not sure if location or path is best, but we use dash separated words everywhere in composer.json, so it'd be best to follow that.

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Aug 29, 2012

Member

Oh also I didn't realize you read it from the main Config instance, that's probably not a good idea because it makes the setting apply to all SVN repos. Ideally the VcsRepository should forward the whole $repoConfig array to VcsDrivers, so that you can get the data from there and configure it like:

{
    "repositories": [
        { "type": "svn", "url": "svn://..", "trunk-path": "boing" }
    ]
}

If you have no idea what I'm talking about, I guess I'll do the change myself while merging the PR :)

Member

Seldaek commented Aug 29, 2012

Oh also I didn't realize you read it from the main Config instance, that's probably not a good idea because it makes the setting apply to all SVN repos. Ideally the VcsRepository should forward the whole $repoConfig array to VcsDrivers, so that you can get the data from there and configure it like:

{
    "repositories": [
        { "type": "svn", "url": "svn://..", "trunk-path": "boing" }
    ]
}

If you have no idea what I'm talking about, I guess I'll do the change myself while merging the PR :)

@bboer

This comment has been minimized.

Show comment Hide comment
@bboer

bboer Aug 29, 2012

Contributor

Hi Seldaek, I'd like to try and implement it the way you are suggesting. To be able to forward the whole repoConfig would it be ok to add the repoConfig as an option to the VcsDriver constructor, or maybe as a parameter on the initialize method of the VcsDriver?

Contributor

bboer commented Aug 29, 2012

Hi Seldaek, I'd like to try and implement it the way you are suggesting. To be able to forward the whole repoConfig would it be ok to add the repoConfig as an option to the VcsDriver constructor, or maybe as a parameter on the initialize method of the VcsDriver?

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Aug 30, 2012

Member

@bboer I think the easiest would be to replace the $url arg of the constructor by the full repoConfig, otherwise you end up sending the url twice.

Member

Seldaek commented Aug 30, 2012

@bboer I think the easiest would be to replace the $url arg of the constructor by the full repoConfig, otherwise you end up sending the url twice.

@bboer

This comment has been minimized.

Show comment Hide comment
@bboer

bboer Aug 30, 2012

Contributor

Hi Seldaek, Just made some changes to pass the repoConfig through the constructor. It is indeed a nicer way to do it like this. Dit some tests locally with an alternative svn structure and it works nicely.

Contributor

bboer commented Aug 30, 2012

Hi Seldaek, Just made some changes to pass the repoConfig through the constructor. It is indeed a nicer way to do it like this. Dit some tests locally with an alternative svn structure and it works nicely.

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Aug 30, 2012

Member

Ok thanks, I'm leaving for a few days and will take a look at merging this when I get back, looks good though at first sight.

Member

Seldaek commented Aug 30, 2012

Ok thanks, I'm leaving for a few days and will take a look at merging this when I get back, looks good though at first sight.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 30, 2012

This pull request fails (merged d1a452b into e2f8098).

This pull request fails (merged d1a452b into e2f8098).

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Aug 30, 2012

Contributor

@bboer please fox the tests for the drivers

Contributor

stof commented Aug 30, 2012

@bboer please fox the tests for the drivers

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 31, 2012

This pull request passes (merged 00361e0 into e2f8098).

This pull request passes (merged 00361e0 into e2f8098).

@nfx

This comment has been minimized.

Show comment Hide comment
@nfx

nfx Oct 18, 2012

Contributor

+1, the sooner the better :)

Contributor

nfx commented Oct 18, 2012

+1, the sooner the better :)

@Seldaek Seldaek merged commit 00361e0 into composer:master Oct 19, 2012

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Oct 19, 2012

Member

Finally merged this, thanks. Also added docs http://getcomposer.org/doc/05-repositories.md#subversion-options

Member

Seldaek commented Oct 19, 2012

Finally merged this, thanks. Also added docs http://getcomposer.org/doc/05-repositories.md#subversion-options

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