genapi: Add support for TagsTagKey #1422
Conversation
justinsb
changed the title
|
I'm going to rebase this, but /assign @acpana |
justinsb
force-pushed
the
genapi_resourcemanager_tags
branch
from
cecf827 to
bcb7dee
Compare
justinsb
changed the title
There was a problem hiding this comment.
/hold for author to merge
/lgtm
/approve
Thanks for rebasing this 🙏🏼 💯
I'd ignore the license-lint failure:
- Fixed it in ci:refactor: reorder steps #1534
- Verified it's not a real issue locally (ran w the fix).
| @@ -46,5 +46,5 @@ type Adapter interface { | |||
|
|
|||
| // Update updates an existing GCP object. | |||
| // This should only be called when Find has previously returned true. | |||
| Update(ctx context.Context) (*unstructured.Unstructured, error) | |||
| Update(ctx context.Context, u *unstructured.Unstructured) error | |||
There was a problem hiding this comment.
just to highlight, we are expecting that implementors will use u to write the status to it, right?
There was a problem hiding this comment.
Right. Specifically we write to the Status. I think in another PR I added more docs, I'll try to track those down!
Also fix up Update method to match Create method. Co-authored-by: alex <[email protected]>
justinsb
force-pushed
the
genapi_resourcemanager_tags
branch
from
f268ede to
a4df877
Compare
|
/lgtm thanks for addressing comments |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: acpana The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Also fix up Update method to match Create method.