-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix grammar, spelling, word casing, whitespace #9132
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: main
Are you sure you want to change the base?
Fix grammar, spelling, word casing, whitespace #9132
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9132 +/- ##
=========================================
Coverage 17.56% 17.56%
- Complexity 15517 15518 +1
=========================================
Files 5906 5906
Lines 528377 528377
Branches 64528 64528
=========================================
+ Hits 92783 92785 +2
+ Misses 425162 425160 -2
Partials 10432 10432
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 immagine I missed a few, and in dutch (intended mistake) these would be considered shouting, but for consistency some sugestions...
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
Outdated
Show resolved
Hide resolved
...rail/src/main/java/org/apache/cloudstack/network/contrail/management/ServiceManagerImpl.java
Outdated
Show resolved
Hide resolved
plugins/network-elements/netscaler/src/main/java/com/cloud/api/commands/StopNetScalerVMCmd.java
Outdated
Show resolved
Hide resolved
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
…ployVMCmd.java Co-authored-by: dahn <[email protected]>
…ployVMCmd.java Co-authored-by: dahn <[email protected]>
Co-authored-by: dahn <[email protected]>
Co-authored-by: dahn <[email protected]>
…ache/cloudstack/network/contrail/management/ServiceManagerImpl.java Co-authored-by: dahn <[email protected]>
…/commands/StopNetScalerVMCmd.java Co-authored-by: dahn <[email protected]>
Co-authored-by: dahn <[email protected]>
throw new CloudRuntimeException("Insufficient capacity", ex); | ||
} | ||
CallContext.current().setEventDetails("Vm Id: " + svm.getId()); | ||
CallContext.current().setEventDetails("VM ID: " + svm.getId()); |
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.
CallContext.current().setEventDetails("VM ID: " + svm.getId()); | |
CallContext.current().setEventDetails("VM Id: " + svm.getId()); |
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.
@FelipeM525 , 'ID' is an acronym for 'identification' according to english rules it should be capatalised. Am I correct @jbampton ?
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 agree with capitalizing both letters, but, regardless of what convention we use (both capitalized or not), there should be a convention. Currently, this PR, that has a focus on fixing word casing inconsistencies, is doing the opposite, by changing some instances of Id
to ID
and leaving others as they are. Either all instances of the word in this PR should be ID
or all should be Id
.
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.
well, agree to disagree. I am happy to conform to conventions, but these are completely unnecessary and gratuit. I am leaving this with the embedded English speaker(s) ;)
plugins/network-elements/netscaler/src/main/java/com/cloud/api/commands/StopNetScalerVMCmd.java
Show resolved
Hide resolved
@blueorangutan package |
@jbampton a [SL] 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 [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11340 |
@blueorangutan package |
@DaanHoogland a [SL] 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 [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11418 |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan LLtest |
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@jbampton a [SL] 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 [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15070 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
@@ -8927,7 +8924,7 @@ public Pair<UserVmVO, Volume> doInTransaction(final TransactionStatus status) th | |||
} | |||
} catch (Exception e) { | |||
logger.debug("Unable to start VM " + vm.getUuid(), e); | |||
CloudRuntimeException ex = new CloudRuntimeException("Unable to start VM with specified id" + e.getMessage()); | |||
CloudRuntimeException ex = new CloudRuntimeException("Unable to start VM with specified ID" + e.getMessage()); |
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.
CloudRuntimeException ex = new CloudRuntimeException("Unable to start VM with specified ID" + e.getMessage()); | |
CloudRuntimeException ex = new CloudRuntimeException("Unable to start VM with specified ID." + e.getMessage()); |
How about that?
@@ -3838,7 +3838,7 @@ public UserVm createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service | |||
if (networkIdList == null || networkIdList.isEmpty()) { | |||
Network networkWithSecurityGroup = _networkModel.getNetworkWithSGWithFreeIPs(owner, zone.getId()); | |||
if (networkWithSecurityGroup == null) { | |||
throw new InvalidParameterValueException("No network with security enabled is found in zone id=" + zone.getUuid()); | |||
throw new InvalidParameterValueException("No network with security enabled is found in zone ID=" + zone.getUuid()); |
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.
throw new InvalidParameterValueException("No network with security enabled is found in zone ID=" + zone.getUuid()); | |
throw new InvalidParameterValueException("No network with security enabled is found in zone ID = " + zone.getUuid()); |
How about that?
@@ -4713,7 +4713,7 @@ private void checkIfHostNameUniqueInNtwkDomain(String hostName, List<? extends N | |||
List<String> hostNames = _vmInstanceDao.listDistinctHostNames(ntwkId); | |||
// * verify that there are no duplicates | |||
if (hostNames.contains(hostName)) { | |||
throw new InvalidParameterValueException("The vm with hostName " + hostName + " already exists in the network domain: " + ntwkDomain.getKey() + "; network=" | |||
throw new InvalidParameterValueException("The VM with hostName " + hostName + " already exists in the network domain: " + ntwkDomain.getKey() + "; network=" |
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.
throw new InvalidParameterValueException("The VM with hostName " + hostName + " already exists in the network domain: " + ntwkDomain.getKey() + "; network=" | |
throw new InvalidParameterValueException("The VM with hostName " + hostName + " already exists in the network domain: " + ntwkDomain.getKey() + "; network = " |
How about that?
@@ -5838,7 +5838,7 @@ private Pod getDestinationPod(Long podId, boolean isRootAdmin) { | |||
} | |||
destinationPod = _podDao.findById(podId); | |||
if (destinationPod == null) { | |||
throw new InvalidParameterValueException("Unable to find the pod to deploy the VM, pod id=" + podId); | |||
throw new InvalidParameterValueException("Unable to find the pod to deploy the VM, pod ID=" + podId); |
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.
throw new InvalidParameterValueException("Unable to find the pod to deploy the VM, pod ID=" + podId); | |
throw new InvalidParameterValueException("Unable to find the pod to deploy the VM, pod ID = " + podId); |
@@ -5853,7 +5853,7 @@ private Cluster getDestinationCluster(Long clusterId, boolean isRootAdmin) { | |||
} | |||
destinationCluster = _clusterDao.findById(clusterId); | |||
if (destinationCluster == null) { | |||
throw new InvalidParameterValueException("Unable to find the cluster to deploy the VM, cluster id=" + clusterId); | |||
throw new InvalidParameterValueException("Unable to find the cluster to deploy the VM, cluster ID=" + clusterId); |
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.
throw new InvalidParameterValueException("Unable to find the cluster to deploy the VM, cluster ID=" + clusterId); | |
throw new InvalidParameterValueException("Unable to find the cluster to deploy the VM, cluster ID = " + clusterId); |
@@ -5868,7 +5868,7 @@ private HostVO getDestinationHost(Long hostId, boolean isRootAdmin, boolean isEx | |||
} | |||
destinationHost = _hostDao.findById(hostId); | |||
if (destinationHost == null) { | |||
throw new InvalidParameterValueException("Unable to find the host to deploy the VM, host id=" + hostId); | |||
throw new InvalidParameterValueException("Unable to find the host to deploy the VM, host ID=" + hostId); |
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.
throw new InvalidParameterValueException("Unable to find the host to deploy the VM, host ID=" + hostId); | |
throw new InvalidParameterValueException("Unable to find the host to deploy the VM, host ID = " + hostId); |
@@ -6935,11 +6935,11 @@ private VMInstanceVO preVmStorageMigrationCheck(Long vmId) { | |||
|
|||
VMInstanceVO vm = _vmInstanceDao.findById(vmId); | |||
if (vm == null) { | |||
throw new InvalidParameterValueException("Unable to find the VM by id=" + vmId); | |||
throw new InvalidParameterValueException("Unable to find the VM by ID=" + vmId); |
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.
throw new InvalidParameterValueException("Unable to find the VM by ID=" + vmId); | |
throw new InvalidParameterValueException("Unable to find the VM by ID = " + vmId); |
@@ -7077,14 +7077,14 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr | |||
|
|||
VMInstanceVO vm = _vmInstanceDao.findById(vmId); | |||
if (vm == null) { | |||
throw new InvalidParameterValueException("Unable to find the VM by id=" + vmId); | |||
throw new InvalidParameterValueException("Unable to find the VM by ID=" + vmId); |
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.
throw new InvalidParameterValueException("Unable to find the VM by ID=" + vmId); | |
throw new InvalidParameterValueException("Unable to find the VM by ID = " + vmId); |
@@ -2049,7 +2049,7 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI | |||
|
|||
_accountMgr.checkAccess(caller, null, true, vmInstance); | |||
|
|||
//Check if its a scale "up" | |||
//Check if it's a scale "up" |
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.
//Check if it's a scale "up" | |
// Check if it's a scale "up" |
[SF] Trillian test result (tid-14376)
|
@daviftorres , you are right, but I think we should merge and create new PRs for any remaining issues. This has been suffering scope creep a lot so far and isn’t functionally crippled. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@FelipeM525 @JoaoJandre please have another look |
Description
This PR cleans up the code base and standardizes some terms for casing.
Fixes in exception messages, log messages, code comments and more.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?