Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fake authorizer takes a tag #144

Merged
merged 7 commits into from Jun 23, 2014
Merged

Fake authorizer takes a tag #144

merged 7 commits into from Jun 23, 2014

Conversation

davecheney
Copy link
Contributor

No description provided.

…rizer-takes-a-tag

Conflicts:
	state/apiserver/agent/agent_test.go
	state/apiserver/root.go
@davecheney
Copy link
Contributor Author

PTAL. I've merged trunk and this is ready for review.

@@ -42,7 +42,7 @@ func NewDeployerAPI(
}
getAuthFunc := func() (common.AuthFunc, error) {
// Get all units of the machine and cache them.
thisMachineTag := authorizer.GetAuthTag()
thisMachineTag := authorizer.GetAuthTag().String()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed to a String() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because getAllUnits takes a string, not a names.Tag ... ah, but getAllUnits is only called from here, and takes the string and turns it into a tag ... fixed.

@wallyworld
Copy link
Member

LGTM with any required changes as per comments. Also, please add a //comment for the deleted test cases where the tag is no longer valid if we intend to come back and rework things or else just delete the line

@davecheney
Copy link
Contributor Author

LGTM with any required changes as per comments. Also, please add a //comment for the deleted test cases where the tag is no longer valid if we intend to come back and rework things or else just delete the line

got it,

@davecheney
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jun 23, 2014

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

jujubot added a commit that referenced this pull request Jun 23, 2014
@jujubot jujubot merged commit a183581 into juju:master Jun 23, 2014
@davecheney davecheney deleted the fake-authorizer-takes-a-tag branch April 13, 2016 23:57
laszlokajtar pushed a commit to laszlokajtar/juju that referenced this pull request Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants