apache
Fix grammar, spelling, word casing, whitespace
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)
- [X] 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
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
Codecov Report
:x: Patch coverage is 6.25000% with 30 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 17.46%. Comparing base (f417c6b) to head (25603ee).
:warning: Report is 44 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #9132 +/- ##
=========================================
Coverage 17.46% 17.46%
Complexity 15516 15516
=========================================
Files 5913 5913
Lines 529385 529385
Branches 64679 64679
=========================================
+ Hits 92448 92450 +2
+ Misses 426518 426517 -1
+ Partials 10419 10418 -1
| Flag | Coverage Δ | |
|---|---|---|
| uitests | 3.58% <ø> (ø) |
|
| unittests | 18.52% <6.25%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@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
@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.
@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.
@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?
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11864
@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 https://github.com/apache/cloudstack/pull/9132#discussion_r1713813784, 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.
@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?
@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?
@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
identificationshould beIDand notId, right? is there anything to discuss here?
If nothing is blocking this PR anymore, no (cc @FelipeM525 )
@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
identificationshould beIDand notId, 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.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
@JoaoJandre , is there any occurance of Id (as opposed to ID) still in this PR? (see https://github.com/apache/cloudstack/pull/9132#issuecomment-2568071452).
If no objection I suggest we run tests again and merge.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
@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 13876
@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.
@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.