-
Notifications
You must be signed in to change notification settings - Fork 43
Add support for ReadyToRemove interface #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1120
Are you sure you want to change the base?
Conversation
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add detail to PR description and to the commit message itself.
For upstream will probably need commit message to describe why this option is being added and what it would be used for.
@@ -0,0 +1,9 @@ | |||
description: > | |||
An indication of whether the inventory is prepared by the system for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Seems like more common term would be "inventory item".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this.
@@ -0,0 +1,9 @@ | |||
description: > | |||
An indication of whether the inventory is prepared by the system for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Interface description seems like it is usually termed as "Implement to..."
(this description reads more like the actual property description).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this
- name: ReadyToRemove | ||
type: boolean | ||
description: > | ||
An indication of whether the item is prepared by the system for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Is this readonly? If not, then I'm wondering about this description. If the property is set to false versus set to true by something?
(The description as written seems to be about what a query of this property would indicate.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be ReadWrite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted
add to approvelist |
5145d4e
to
00ebe9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me
Let's give this a week or two upstream before we take it downstream |
00ebe9e
to
3703bb8
Compare
Please add
-- |
This new D-Bus interface is implemented to indicate whether the item is prepared by the system for removal. This property shall indicate whether the system is prepared for the removal of the item. This matches the Redfish ReadyToRemove interface. It will be on other devices. Change-Id: I5c50f8792d968e3427e64ebf99a2fc5f32aaa87a Signed-off-by: Abiola Asojo <[email protected]>
3703bb8
to
6c4557e
Compare
I would suggest to wait on this until we conclude the Concurrent Maintenance backend design. Is there anything else at bmcweb which needs this dbus interface early? |
Add support for ReadyToRemove interface.