Merge lp://staging/~allenap/gwacl/update-role into lp://staging/gwacl

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: 184
Merged at revision: 184
Proposed branch: lp://staging/~allenap/gwacl/update-role
Merge into: lp://staging/gwacl
Diff against target: 76 lines (+55/-0)
2 files modified
management_base.go (+30/-0)
management_base_test.go (+25/-0)
To merge this branch: bzr merge lp://staging/~allenap/gwacl/update-role
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+175048@code.staging.launchpad.net

Commit message

New API method UpdateRole.

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :

Looks good.

[0]

14 + RoleName string
15 + PersistentVMRole *PersistentVMRole

The PersistentVMRole object already contains the RoleName.

[1]

"alas, struct embedding is too clunky, so copy-n-paste it is"

:/

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

> [0]
>
> 14 + RoleName string
> 15 + PersistentVMRole *PersistentVMRole
>
> The PersistentVMRole object already contains the RoleName.

There's nothing in the docs to say that you can't change the name of the role by specifying a different name. In other words, PersistentVMRole defines the role resource, and ServiceName, DeploymentName and RoleName define the resource's location.

Thanks for the review :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: