Skip to content

Compute v2: Move ServerTagsExt to servers.TagsExt#1760

Merged
jtopjian merged 1 commit intogophercloud:masterfrom
jtopjian:computev2-tags-fix
Oct 24, 2019
Merged

Compute v2: Move ServerTagsExt to servers.TagsExt#1760
jtopjian merged 1 commit intogophercloud:masterfrom
jtopjian:computev2-tags-fix

Conversation

@jtopjian
Copy link
Copy Markdown
Contributor

This commit moves the tags extension's ServerTagsExt to
the servers package. While this is slighly nuanced, the tags
extension can be thought of to work directly on a server's
tags and the ability to extract a server's tags can be part
of the servers package since the original ExtractTags
microversion support already exists.

For #1669

This commit moves the tags extension's ServerTagsExt to
the servers package. While this is slighly nuanced, the tags
extension can be thought of to work directly on a server's
tags and the ability to extract a server's tags can be part
of the servers package since the original ExtractTags
microversion support already exists.
@jtopjian jtopjian requested a review from ozerovandrei October 24, 2019 02:08
@jtopjian
Copy link
Copy Markdown
Contributor Author

@ozerovandrei after looking at

// ServerTagsExt is an extension to the base Server object.
type ServerTagsExt struct {
// Tags contains a list of server tags.
Tags []string `json:"tags"`
}

I see the benefit to having a struct extension like you made since there is not an easy way to see the tags from a list of servers. But since we already have the ability to extract tags from a single server result in microversions.go, I think the struct extension is better located in the servers package.

Like the commit message says, this is slightly nuanced / nit picking, so I'm open to discussion on this.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 76.929% when pulling b74fd26 on jtopjian:computev2-tags-fix into 0907b32 on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Oct 24, 2019

Build succeeded.

Copy link
Copy Markdown
Contributor

@ozerovandrei ozerovandrei left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for fixing this!

@jtopjian jtopjian merged commit ee22944 into gophercloud:master Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants