Fix grammar, spelling, word casing, whitespace #9132

jbampton · 2024-05-28T11:08:33Z

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

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

codecov · 2024-05-28T11:17:22Z

Codecov Report

Attention: Patch coverage is 5.88235% with 32 lines in your changes missing coverage. Please review.

Project coverage is 16.06%. Comparing base (b1ba9bf) to head (02d59c0).
Report is 32 commits behind head on main.

Files with missing lines	Patch %	Lines
...he/cloudstack/api/command/user/vm/DeployVMCmd.java	0.00%	3 Missing ⚠️
...stack/engine/orchestration/VolumeOrchestrator.java	0.00%	2 Missing ⚠️
...command/admin/internallb/StartInternalLBVMCmd.java	0.00%	1 Missing ⚠️
.../command/admin/internallb/StopInternalLBVMCmd.java	0.00%	1 Missing ⚠️
...api/command/admin/systemvm/DestroySystemVmCmd.java	0.00%	1 Missing ⚠️
.../api/command/admin/systemvm/RebootSystemVmCmd.java	0.00%	1 Missing ⚠️
...k/api/command/admin/systemvm/ScaleSystemVMCmd.java	0.00%	1 Missing ⚠️
...k/api/command/admin/systemvm/StartSystemVMCmd.java	0.00%	1 Missing ⚠️
...ck/api/command/admin/systemvm/StopSystemVmCmd.java	0.00%	1 Missing ⚠️
...api/command/admin/systemvm/UpgradeSystemVMCmd.java	0.00%	1 Missing ⚠️
... and 19 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9132      +/-   ##
============================================
- Coverage     16.06%   16.06%   -0.01%     
+ Complexity    12872    12871       -1     
============================================
  Files          5642     5642              
  Lines        493953   493953              
  Branches      59889    59889              
============================================
- Hits          79351    79350       -1     
- Misses       405817   405818       +1     
  Partials       8785     8785

Flag	Coverage Δ
uitests	`4.01% <ø> (ø)`
unittests	`16.90% <5.88%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DaanHoogland

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

...rail/src/main/java/org/apache/cloudstack/network/contrail/management/ServiceManagerImpl.java

plugins/network-elements/netscaler/src/main/java/com/cloud/api/commands/StopNetScalerVMCmd.java

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

server/src/test/java/com/cloud/vm/UserVmManagerTest.java

github-actions · 2024-06-14T08:31:38Z

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 <daan.hoogland@gmail.com>

Co-authored-by: dahn <daan.hoogland@gmail.com>

…ache/cloudstack/network/contrail/management/ServiceManagerImpl.java Co-authored-by: dahn <daan.hoogland@gmail.com>

…/commands/StopNetScalerVMCmd.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Co-authored-by: dahn <daan.hoogland@gmail.com>

FelipeM525 · 2024-08-02T13:36:48Z

...rail/src/main/java/org/apache/cloudstack/network/contrail/management/ServiceManagerImpl.java

@@ -134,7 +134,7 @@ private ServiceVirtualMachine createServiceVM(DataCenter zone, Account owner, Vi
        } catch (InsufficientCapacityException ex) {
            throw new CloudRuntimeException("Insufficient capacity", ex);
        }
-        CallContext.current().setEventDetails("Vm Id: " + svm.getId());
+        CallContext.current().setEventDetails("VM ID: " + svm.getId());


Suggested change

CallContext.current().setEventDetails("VM ID: " + svm.getId());

CallContext.current().setEventDetails("VM Id: " + svm.getId());

@FelipeM525 , 'ID' is an acronym for 'identification' according to english rules it should be capatalised. Am I correct @jbampton ?

@DaanHoogland

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.

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) ;)

server/src/test/java/com/cloud/vm/UserVmManagerTest.java

plugins/network-elements/netscaler/src/main/java/com/cloud/api/commands/StopNetScalerVMCmd.java

jbampton · 2024-10-14T15:35:27Z

@blueorangutan package

blueorangutan · 2024-10-14T15:36:03Z

@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.

blueorangutan · 2024-10-14T16:39:47Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11340

DaanHoogland · 2024-10-23T11:58:46Z

@blueorangutan package

blueorangutan · 2024-10-23T12:00:04Z

@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.

blueorangutan · 2024-10-23T12:49:42Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11418

tools/git/git-pr

github-actions · 2024-12-20T13:39:36Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

DaanHoogland · 2024-12-20T13:47:10Z

@blueorangutan package

blueorangutan · 2024-12-20T13:48:04Z

@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.

DaanHoogland · 2024-12-20T13:49:35Z

@GaOrtiga @FelipeM525 @JoaoJandre , @jbampton is the only native english speaker active on this ticket. As contributor we have are specialities and as coders we get to decide on coding stardards, but not on English language standards, so I suggest we go with @jbampton 's suggestions where it come to log messages. What do you guys think?

blueorangutan · 2024-12-20T15:18:04Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11864

tools/git/git-pr

JoaoJandre · 2024-12-23T11:00:25Z

@GaOrtiga @FelipeM525 @JoaoJandre , @jbampton is the only native english speaker active on this ticket. As contributor we have are specialities and as coders we get to decide on coding stardards, but not on English language standards, so I suggest we go with @jbampton 's suggestions where it come to log messages. What do you guys think?

@DaanHoogland, what @FelipeM525 and @GaOrtiga were pointing out in this comment #9132 (comment), is that, in some changed lines, the abbreviation (it is not an acronym) ID is left as Id (such as in line 106 of api/src/main/java/org/apache/cloudstack/api/command/admin/internallb/StartInternalLBVMCmd.java), whereas in other lines, it is changed to ID (such as line 137 of plugins/network-elements/juniper-contrail/src/main/java/org/apache/cloudstack/network/contrail/management/ServiceManagerImpl.java).

I agree that the correct abbreviation is ID, and not Id, and thus, my suggestion is to change all occurrences of Id to ID (at least in the lines that this PR is touching).

In any case, I would refrain from going for "argumentum ad verecundiam", as this is not a healthy way of solving conflicts in a community. @jbampton may be a native speaker, but he is not perfect, and non-native speakers may catch errors in his English nonetheless.

DaanHoogland · 2024-12-31T12:10:47Z

@jbampton (cc @JoaoJandre ) is there anything to discuss on this or a guideline to codify somewhere, still?

Is it maybe possible to split this PR into smaller chunks so we can merge the parts where no discussion is being held about?

JoaoJandre · 2025-01-02T12:18:36Z

@jbampton (cc @JoaoJandre ) is there anything to discuss on this or a guideline to codify somewhere, still?

Is it maybe possible to split this PR into smaller chunks so we can merge the parts where no discussion is being held about?

@DaanHoogland I think we all agree that the abbreviation for identification should be ID and not Id, right? is there anything to discuss here?

DaanHoogland · 2025-01-02T14:39:29Z

@jbampton (cc @JoaoJandre ) is there anything to discuss on this or a guideline to codify somewhere, still?
Is it maybe possible to split this PR into smaller chunks so we can merge the parts where no discussion is being held about?

@DaanHoogland I think we all agree that the abbreviation for identification should be ID and not Id, right? is there anything to discuss here?

If nothing is blocking this PR anymore, no (cc @FelipeM525 )

JoaoJandre · 2025-01-02T16:49:56Z

@jbampton (cc @JoaoJandre ) is there anything to discuss on this or a guideline to codify somewhere, still?
Is it maybe possible to split this PR into smaller chunks so we can merge the parts where no discussion is being held about?

@DaanHoogland I think we all agree that the abbreviation for identification should be ID and not Id, right? is there anything to discuss here?

If nothing is blocking this PR anymore, no (cc @FelipeM525 )

@DaanHoogland , in my last message I pointed out that there are parts of the PR where ID is written as "ID", and some other parts where it is written as "Id". I mean, that is what we all (me, @FelipeM525 and @GaOrtiga) have been pointing out so far. We are trying to say that the author should fix it.

github-actions · 2025-01-08T13:23:19Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

Fix grammar, spelling, word casing, whitespace

b28eccb

boring-cyborg bot added component:api component:database component:networking component:orchestration labels May 28, 2024

DaanHoogland reviewed May 28, 2024

View reviewed changes

github-actions bot added the status:has-conflicts label Jun 14, 2024

jbampton and others added 7 commits July 10, 2024 21:05

Update api/src/main/java/org/apache/cloudstack/api/command/user/vm/De…

3574cdc

…ployVMCmd.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Update api/src/main/java/org/apache/cloudstack/api/command/user/vm/De…

fba6bf5

…ployVMCmd.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Update server/src/main/java/com/cloud/network/NetworkServiceImpl.java

287b40f

Co-authored-by: dahn <daan.hoogland@gmail.com>

Update server/src/test/java/com/cloud/vm/UserVmManagerTest.java

79a8b92

Co-authored-by: dahn <daan.hoogland@gmail.com>

Update plugins/network-elements/juniper-contrail/src/main/java/org/ap…

8ac51b7

…ache/cloudstack/network/contrail/management/ServiceManagerImpl.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Update plugins/network-elements/netscaler/src/main/java/com/cloud/api…

0c81a31

…/commands/StopNetScalerVMCmd.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Update server/src/test/java/com/cloud/vm/UserVmManagerTest.java

177d355

Co-authored-by: dahn <daan.hoogland@gmail.com>

FelipeM525 suggested changes Aug 2, 2024

View reviewed changes

jbampton marked this pull request as draft October 14, 2024 12:46

Merge branch 'main' into fix-spelling-grammar-case-whitespace

f104bd5

github-actions bot removed the status:has-conflicts label Oct 14, 2024

DaanHoogland reviewed Oct 24, 2024

View reviewed changes

tools/git/git-pr Outdated Show resolved Hide resolved

github-actions bot added the status:has-conflicts label Dec 20, 2024

Merge branch 'main' into fix-spelling-grammar-case-whitespace

403b87d

github-actions bot removed the status:has-conflicts label Dec 20, 2024

jbampton commented Dec 20, 2024

View reviewed changes

tools/git/git-pr Outdated Show resolved Hide resolved

jbampton added 2 commits December 21, 2024 08:22

Update tools/git/git-pr

e9b5c9d

Merge branch 'main' into fix-spelling-grammar-case-whitespace

02d59c0

jbampton mentioned this pull request Dec 31, 2024

misc: fix spelling #8272

Draft

13 tasks

DaanHoogland mentioned this pull request Dec 31, 2024

Improve logging to include more identifiable information #9873

Merged

9 tasks

github-actions bot added the status:has-conflicts label Jan 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix grammar, spelling, word casing, whitespace #9132

Fix grammar, spelling, word casing, whitespace #9132

jbampton commented May 28, 2024

codecov bot commented May 28, 2024 •

edited

Loading

DaanHoogland left a comment

github-actions bot commented Jun 14, 2024

FelipeM525 Aug 2, 2024

DaanHoogland Aug 12, 2024

GaOrtiga Aug 12, 2024

DaanHoogland Aug 12, 2024

jbampton commented Oct 14, 2024

blueorangutan commented Oct 14, 2024

blueorangutan commented Oct 14, 2024

DaanHoogland commented Oct 23, 2024

blueorangutan commented Oct 23, 2024

blueorangutan commented Oct 23, 2024

github-actions bot commented Dec 20, 2024

DaanHoogland commented Dec 20, 2024

blueorangutan commented Dec 20, 2024

DaanHoogland commented Dec 20, 2024

blueorangutan commented Dec 20, 2024

JoaoJandre commented Dec 23, 2024

DaanHoogland commented Dec 31, 2024

JoaoJandre commented Jan 2, 2025

DaanHoogland commented Jan 2, 2025

JoaoJandre commented Jan 2, 2025 •

edited

Loading

github-actions bot commented Jan 8, 2025

	CallContext.current().setEventDetails("VM ID: " + svm.getId());
	CallContext.current().setEventDetails("VM Id: " + svm.getId());

Fix grammar, spelling, word casing, whitespace #9132

Are you sure you want to change the base?

Fix grammar, spelling, word casing, whitespace #9132

Conversation

jbampton commented May 28, 2024

Description

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?

codecov bot commented May 28, 2024 • edited Loading

Codecov Report

DaanHoogland left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 14, 2024

FelipeM525 Aug 2, 2024

Choose a reason for hiding this comment

DaanHoogland Aug 12, 2024

Choose a reason for hiding this comment

GaOrtiga Aug 12, 2024

Choose a reason for hiding this comment

DaanHoogland Aug 12, 2024

Choose a reason for hiding this comment

jbampton commented Oct 14, 2024

blueorangutan commented Oct 14, 2024

blueorangutan commented Oct 14, 2024

DaanHoogland commented Oct 23, 2024

blueorangutan commented Oct 23, 2024

blueorangutan commented Oct 23, 2024

github-actions bot commented Dec 20, 2024

DaanHoogland commented Dec 20, 2024

blueorangutan commented Dec 20, 2024

DaanHoogland commented Dec 20, 2024

blueorangutan commented Dec 20, 2024

JoaoJandre commented Dec 23, 2024

DaanHoogland commented Dec 31, 2024

JoaoJandre commented Jan 2, 2025

DaanHoogland commented Jan 2, 2025

JoaoJandre commented Jan 2, 2025 • edited Loading

github-actions bot commented Jan 8, 2025

codecov bot commented May 28, 2024 •

edited

Loading

JoaoJandre commented Jan 2, 2025 •

edited

Loading