Fix: The metrics view API response is not super-set of resources response keys#3834
Conversation
| } | ||
|
|
||
| public Boolean isDestroyed() { | ||
| public Boolean getDestroyed() { |
There was a problem hiding this comment.
boolean getter should adhere to is() pattern
|
I do not care much either way but we agreed on the getter convention for booleans a while back. thoughts @rhtyd ? |
|
@DaanHoogland my suggestion is, we can use lombok. |
|
@weizhouapache looks cool. @Pearl1594 @rhtyd is there a specific reason to go back on our getter convention, or any other reason we shouldn't use lombok? (name of the neightbourhood I live in;) after the indonesian isleland \o/) |
|
@Pearl1594 @rhtyd can you meddle in on @weizhouapache 's idea? (lombok would actually make this a get<>() as it is Boolean and not boolean) |
|
@DaanHoogland I've reverted the particular function to it's original naming convention. the getter function need not be modified for this case. |
|
That's how BeanUtils::copyProperties and most IDEs works. The problem of booleans having |
|
@DaanHoogland this is a special case for the metrics apis that extend over resource list apis. Without using getters instead of issers, it cannot be fixed. Turns out the previously referred agreement is merely our own preference but not a standard convention for IDEs, and libraries. |
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-658 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
code lgtm, let's discuss lombok on an issue page if appropriate.
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-681 |
|
@blueorangutan test |
1 similar comment
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-836)
|
…apache#3834) The metrics API has few properties missing that are present in the corresponding resource. Fixes apache#3831 Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> Co-authored-by: Rohit Yadav <rohit@apache.org>
6 participants
Description
The metrics API has few properties missing that are present in the corresponding resource. This addressed the following issue: #3831
Types of changes