/juju

apiserver: remove dependency on resource/api #4673

Merged
merged 1 commit into from Mar 10, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@davecheney @jujubot
@davecheney
Contributor

davecheney commented Mar 10, 2016

The apiserver was importing resource/api to get one constant to
construct a URL path. This made the apiserver depend on all the code
that the resource/api package depends on, and flirted with an import
loop.

Given that the line directly below the one affected by this change does
the same thing, but without a constant in another package, it seems
simpler to remove the dependency and just write the literal string in
the one place it is used, and has test coverage.

(Review request: http://reviews.vapour.ws/r/4112/)
@davecheney
apiserver: remove dependency on resource/api 
The apiserver was importing resource/api to get one constant to
construct a URL path. This made the apiserver depend on all the code
that the resource/api package depends on, and flirted with an import
loop.

Given that the line directly below the one affected by this change does
the same thing, but without a constant in another package, it seems
simpler to remove the dependency and just write the literal string in
the one place it is used, and has test coverage.
769a40c
@davecheney
Contributor

davecheney commented Mar 10, 2016

$$JFDI$$
@jujubot
Contributor

jujubot commented Mar 10, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jujubot added a commit that referenced this pull request Mar 10, 2016

@jujubot
Merge pull request #4673 from davecheney/apiserver-remove-resource-ap… 
…i-dependency

apiserver: remove dependency on resource/api

The apiserver was importing resource/api to get one constant to
construct a URL path. This made the apiserver depend on all the code
that the resource/api package depends on, and flirted with an import
loop.

Given that the line directly below the one affected by this change does
the same thing, but without a constant in another package, it seems
simpler to remove the dependency and just write the literal string in
the one place it is used, and has test coverage.

(Review request: http://reviews.vapour.ws/r/4112/)
61607ab

@jujubot jujubot merged commit 61607ab into juju:master Mar 10, 2016

@davecheney davecheney deleted the davecheney:apiserver-remove-resource-api-dependency branch Mar 10, 2016

ericsnowcurrently added a commit to ericsnowcurrently/juju that referenced this pull request Mar 17, 2016

@ericsnowcurrently
Revert "Merge pull request #4673 from davecheney/apiserver-remove-res… 
…ource-api-dependency"

This reverts commit 61607ab, reversing
changes made to 59b80a4.

Conflicts:
	apiserver/apiserver.go
18d8e31

ericsnowcurrently added a commit to ericsnowcurrently/juju that referenced this pull request Mar 17, 2016

@ericsnowcurrently
Revert "Merge pull request #4673 from davecheney/apiserver-remove-res… 
…ource-api-dependency"

This reverts commit 61607ab, reversing
changes made to 59b80a4.
84557d3

ericsnowcurrently added a commit to ericsnowcurrently/juju that referenced this pull request Mar 21, 2016

@ericsnowcurrently
Revert "Merge pull request #4673 from davecheney/apiserver-remove-res… 
…ource-api-dependency"

This reverts commit 61607ab, reversing
changes made to 59b80a4.
9795695

ericsnowcurrently added a commit to ericsnowcurrently/juju that referenced this pull request Mar 29, 2016

@ericsnowcurrently
Revert "Merge pull request #4673 from davecheney/apiserver-remove-res… 
…ource-api-dependency"

This reverts commit 61607ab, reversing
changes made to 59b80a4.
02e8e4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment