CLOUDSTACK-10365: Change the "getXXX" boolean-related method names to…#2847
CLOUDSTACK-10365: Change the "getXXX" boolean-related method names to…#2847DaanHoogland merged 1 commit intoapache:masterapache/cloudstack:masterfrom Kui-Liu:masterCopy head branch name to clipboard
Conversation
|
@blueorangutan package |
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2301 |
|
@blueorangutan test |
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
DaanHoogland
left a comment
There was a problem hiding this comment.
code looks fine. let's give this some priority as it keeps getting pushed forward. @rafaelweingartner @rhtyd ....
rafaelweingartner
left a comment
There was a problem hiding this comment.
@brucekuiliu Thanks for the PR.
One note though. Please, do not do the same in the future. By the "same" here, I mean, this is the third to address the same issue that you created. The first one was closed, then the second one you deleted the branch before it actually being merged. This breaks history, and it makes it harder for us to review it. I had to revisit all of the other to see the comments there. For instance, there was a concern from Frank regarding the change in these method names that was never answered.
|
@brucekuiliu I agree with @rafaelweingartner as I noted in your prior PR, please have some patience with us, we are not the fasted mergers in the world of github. |
| @Override | ||
| public boolean getIsPersistent() { | ||
| public boolean isPersistent() { | ||
| return isPersistent; |
There was a problem hiding this comment.
This cause the GC bug, our DB framework assumes that the attributes (map) is created by a method interceptor so the name of the variable should not have the 'is', the name of the variable that related to a db table's column should be something after removing get/is string prefix: https://github.com/apache/cloudstack/blob/master/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java#L480
/cc @nvazquez @anuragaw @sh0wstopper @dhlaluku #2935
… "isXXX".
These boolean-return methods are named as "getXXX".
Other boolean-return methods are named as "isXXX".
Considering there methods will return boolean values, it should be more clear and consistent to rename them as "isXXX".
(rebase #2602 and #2816)