Cleanup APIs getCommandName - #7022
Conversation
|
@ezntt , good initiative. What do you propose we do with the remaining 88? |
| return cmdName; | ||
| } | ||
|
|
||
| public static String getResponseNameByClass(Class<?> clazz) { |
There was a problem hiding this comment.
@ezntt , good initiative. What do you propose we do with the remaining 88?
As the PR is intended to clean up the repeated code (it will not change the APIs behavior), I would rather not touch those 88 APIs here. My proposal for those remaining 88 APIs is a later discussion on how we should handle them and if changing them would affect any other place.
is this being used?
Yes, it is used in class UploadListener.java:
public static final Map<String, String> responseNameMap;
static {
Map<String, String> tempMap = new HashMap<String, String>();
tempMap.put(Type.ISO.toString(), BaseCmd.getResponseNameByClass(ExtractIsoCmd.class));
tempMap.put(Type.TEMPLATE.toString(), BaseCmd.getResponseNameByClass(ExtractTemplateCmd.class));
tempMap.put(Type.VOLUME.toString(), BaseCmd.getResponseNameByClass(ExtractVolumeCmd.class));
tempMap.put("DEFAULT", "extractresponse");
responseNameMap = Collections.unmodifiableMap(tempMap);
}|
@ezntt please note, most actions fail. Maybe some compilation issue? |
also change 2 missing APIs
Codecov Report
@@ Coverage Diff @@
## main #7022 +/- ##
============================================
+ Coverage 11.31% 11.54% +0.22%
- Complexity 7334 7510 +176
============================================
Files 2494 2494
Lines 246787 246680 -107
Branches 38533 38534 +1
============================================
+ Hits 27933 28471 +538
+ Misses 215255 214528 -727
- Partials 3599 3681 +82
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@blueorangutan package |
|
@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5126 |
|
@blueorangutan test |
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-5691)
|
|
@ezntt are you done with the changes you want for this PR? |
@DaanHoogland yes, I'm done with these changes |
|
@blueorangutan test matrix |
|
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
@blueorangutan test matrix |
|
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
@weizhouapache do you approve now? |
|
Trillian test result (tid-5724)
|
|
Trillian test result (tid-5727)
|
|
Trillian test result (tid-5725)
|
|
Trillian test result (tid-5728)
|
|
Trillian test result (tid-5726)
|
|
Trillian test result (tid-5729)
|
GutoVeronezi
left a comment
There was a problem hiding this comment.
CLGTM
Nice clean up, @ezntt.
|
Kudos, SonarCloud Quality Gate passed! |
apache/cloudstack#7022 simplified API class implementation. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
apache/cloudstack#7022 simplified API class implementation. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Description
It was noticed that the APIs (most of them) follow a pattern in which the response name equals to
APINAME.toLowerCase() + 'response', which is the value returned bygetCommandNamemethod.Therefore, I am proposing the removal of the different repeated
getCommandNameinstructions throughout the APIs. Additionally, it also removed API name/response name declarations from the affected classes; the API name values were moved to thenameparameter in@APICommand.To help me in the process of normalizing the code base, I created a script that iterates through the APIs using the existing method in class
ApiXmlDocWriter. The number of APIs that did not follow the standard is 88, so they weren't changed, and they can be seen in the collapse below with their respective response name and how they would be if they were in the majority standard:Unchanged APIs
Types of changes
How Has This Been Tested?
After the changes, I've built the project locally and succeeded.