X Tutup
Skip to content

compute: API for server actions#67

Merged
olivergondza merged 4 commits intoopenstack4j:masterfrom
mrsujitsah:pApi
Aug 27, 2020
Merged

compute: API for server actions#67
olivergondza merged 4 commits intoopenstack4j:masterfrom
mrsujitsah:pApi

Conversation

@mrsujitsah
Copy link
Copy Markdown
Contributor

@mrsujitsah mrsujitsah commented Jul 18, 2020

I have written the API for implementation of server event actions. This API fetch sever event actions

@mrsujitsah
Copy link
Copy Markdown
Contributor Author

mrsujitsah commented Jul 20, 2020

@olivergondza please review

olivergondza
olivergondza requested changes Aug 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@olivergondza olivergondza left a comment

Choose a reason for hiding this comment

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

Sorry I am getting to this with delay. IIUC, this is new api for https://docs.openstack.org/api-ref/compute/index.html#servers-actions-servers-os-instance-actions.

It seems all my comments are related to terminology where I would like to stick with the naming of the API for easier understanding.

olivergondza
olivergondza reviewed Aug 10, 2020
View reviewed changes
core/src/main/java/org/openstack4j/model/compute/ServerAction.java Outdated
import org.openstack4j.model.common.ServerActionEvent;

/**
* This interface describes the getter-methods (and thus components) of a Event.
Copy link
Copy Markdown
Member

@olivergondza olivergondza Aug 10, 2020

Choose a reason for hiding this comment

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

It is ServerAction now. Better to refer to symbols using {@link ...} to catch these errors.

@olivergondza
Copy link
Copy Markdown
Member

olivergondza commented Aug 10, 2020

I see the naming is easier to understand now. Thanks!

However, I would prefer to do these to make it internally consistent:

EventServices -> ServerActionsService
EventServicesImpl -> NovaServerActionsService
EventListBuilder -> ServerActionEventBuilder
GenericEventsList -> NovaServerActionEvent
GenericEventsList.EventConcreteBuilder -> NovaServerActionEvent.NovaServerActionEventBuilder (or just NovaServerActionEvent.Builder)
ServerEvent -> NovaServerAction

Also, the package org.openstack4j.model.common is for the model objects shared amongs services - which is not the case for any of the classes added. It either belongs to org.openstack4j.openstack.compute.domain or org.openstack4j.model.compute.

@olivergondza olivergondza merged commit 9a0d006 into openstack4j:master Aug 27, 2020
@olivergondza
Copy link
Copy Markdown
Member

olivergondza commented Aug 27, 2020

Merged and amended with 019a63a. Thanks!

@olivergondza olivergondza mentioned this pull request Aug 27, 2020
Closed
@olivergondza olivergondza changed the title API for server event actions compute: API for server actions Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olivergondza olivergondza Awaiting requested review from olivergondza

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mrsujitsah @olivergondza
X Tutup