Conversation
|
Build succeeded.
|
| ) | ||
|
|
||
| // ListInstanceActions makes a request against the API to list the servers actions. | ||
| func ListInstanceActions(client *gophercloud.ServiceClient, id string) pagination.Pager { |
There was a problem hiding this comment.
Since this is under its own package (instanceactions), we can rename this to just List.
| } | ||
|
|
||
| // ListInstanceActions makes a request against the API to get a server action. | ||
| func GetInstanceAction(client *gophercloud.ServiceClient, serverID, requestID string) (r InstanceActionResult) { |
| Message *string `json:"message"` | ||
| ProjectID string `json:"project_id"` | ||
| RequestID string `json:"request_id"` | ||
| StartTime time.Time `json:"start_time"` |
There was a problem hiding this comment.
The JSON tag should be -:
StartTime time.Time `json:"-"`
This will resolve the Travis error, too.
|
|
||
| // ExtractInstanceAction interprets the results of a single page from a | ||
| // GetInstanceActiones() call, producing a map of instanceAction. | ||
| func ExtractInstanceAction(r pagination.Page) (InstanceAction, error) { |
There was a problem hiding this comment.
I think this function can safely be removed.
|
|
||
| // ExtractInstanceActions interprets the results of a single page from a | ||
| // ListInstanceActiones() call, producing a map of instanceActions. | ||
| func ExtractInstanceActions(r pagination.Page) ([]InstanceAction, error) { |
There was a problem hiding this comment.
Even though we're renaming some of the other functions (for example, ListInstanceActions to List), this function stays ExtractInstanceActions. It's just weird naming pattern of Gophercloud.
| type InstanceAction struct { | ||
| Action string `json:"action"` | ||
| InstanceUUID string `json:"instance_uuid"` | ||
| Message *string `json:"message"` |
There was a problem hiding this comment.
Having this as *string will cause the value to be nil if there is no message. Is there a reason to do this instead of having an empty value be ""? If there is, I'm OK with keeping this as *string.
There was a problem hiding this comment.
It permits to say to the dev that the field be empty.
If it's a pointer is easier for the dev to konw the field is optional and must be checked before used.
70a4f48 to
b43ad7f
Compare
|
Build succeeded.
|
|
Commit of review squashed : |
For #1845
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://docs.openstack.org/api-ref/compute/?expanded=list-actions-for-server-detail
https://github.com/openstack/nova/blob/51ed40a6a5fc046cef35337980a1fc5ad704a421/nova/api/openstack/compute/instance_actions.py#L83