Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
erikbocks commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3847884282 @DaanHoogland @JoaoJandre, I agree with João's POV, and think it is better to close it for now, and maybe re-implement it in the future with the change in the API's as well. Thank you for the reviews. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
erikbocks closed pull request #11819: Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd URL: https://github.com/apache/cloudstack/pull/11819 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3847478944 I have approved but have no big stake in it. @erikbocks you alreight to close this unmerged, and redesign/reimplement? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
JoaoJandre commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3847390435 > @JoaoJandre @erikbocks @bernardodemarco , will we abandon this or push it through? @DaanHoogland I dislike this change and would prefer if it was abandoned, but I understand others might disagree. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
bernardodemarco commented on code in PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#discussion_r2746266967 ## api/src/main/java/org/apache/cloudstack/api/command/admin/autoscale/CreateAutoScaleCounterCmd.java: ## @@ -34,7 +34,7 @@ @APICommand(name = "createCounter", description = "Adds metric counter for VM auto scaling", responseObject = CounterResponse.class, Review Comment: @erikbocks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3822788678 @JoaoJandre @erikbocks @bernardodemarco , will we abandon this or push it through? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
harikrishna-patnala commented on code in PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#discussion_r2744746111 ## api/src/main/java/org/apache/cloudstack/api/command/admin/autoscale/CreateAutoScaleCounterCmd.java: ## @@ -34,7 +34,7 @@ @APICommand(name = "createCounter", description = "Adds metric counter for VM auto scaling", responseObject = CounterResponse.class, Review Comment: If this the case, I think it is better to mark the existing one as deprecated and use new API class extending createCounter. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
bernardodemarco commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3725604975 @erikbocks, are there any updates regarding these comments/suggestions https://github.com/apache/cloudstack/pull/11819#discussion_r2489232189 and https://github.com/apache/cloudstack/pull/11819#issuecomment-3485451128? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
github-actions[bot] commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3674736297 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
JoaoJandre commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3485451128 While I agree that the API name is bad, I don't like this change as it is easier to find a command class that has the same name as the API. Since the API name will stay the same, I think that this PR actually makes navigating through the code harder. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on code in PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#discussion_r2489232189 ## api/src/main/java/org/apache/cloudstack/api/command/admin/autoscale/CreateAutoScaleCounterCmd.java: ## @@ -34,7 +34,7 @@ @APICommand(name = "createCounter", description = "Adds metric counter for VM auto scaling", responseObject = CounterResponse.class, Review Comment: ```suggestion @APICommand(name = "createAutoScaleCounter", description = "Adds metric counter for VM auto scaling", responseObject = CounterResponse.class, ``` this needs to change for the API name to change @erikbocks . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3473833325 @erikbocks this is good to go but please be aware that this does nothing. The public names for the APIs do not change. Were you intending for those to change? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3467545687 [SF] Trillian test result (tid-14740) Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8 Total time taken: 64654 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11819-t14740-kvm-ol8.zip Smoke tests completed. 147 look OK, 1 have errors, 1 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_02_enableHumanReadableLogs | `Error` | 0.31 | test_human_readable_logs.py all_test_image_store_object_migration | `Skipped` | --- | test_image_store_object_migration.py -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3467191815 [SF] Trillian Build Failed (tid-14749) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3460517772 [SF] Trillian Build Failed (tid-14738) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3460508396 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3460505200 @blueorangutan test keepEnv -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3456832551 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15577 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
blueorangutan commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3456449893 @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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3456447393 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
weizhouapache commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3456371484 > @shwstppr @weizhouapache , can we continue with this or are there any impediments? we can continue -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3456355592 @shwstppr @weizhouapache , can we continue with this or are there any impediments? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
erikbocks commented on code in PR #11819:
URL: https://github.com/apache/cloudstack/pull/11819#discussion_r2466546892
##
server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java:
##
@@ -1527,7 +1527,7 @@ public Condition createCondition(CreateConditionCmd cmd) {
}
@Override
-public List listCounters(ListCountersCmd cmd) {
Review Comment:
I think that the current renaming brought a good meaningfulness to the
classes' names. Do you wish me to rename other classes or to revert some of the
changed ones?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
weizhouapache commented on code in PR #11819:
URL: https://github.com/apache/cloudstack/pull/11819#discussion_r2465943756
##
api/src/main/java/org/apache/cloudstack/api/command/admin/autoscale/DeleteAutoScaleConditionCounterCmd.java:
##
@@ -34,7 +34,7 @@
@APICommand(name = "deleteCounter", description = "Deletes a counter for VM
auto scaling", responseObject = SuccessResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
-public class DeleteCounterCmd extends BaseAsyncCmd {
+public class DeleteAutoScaleConditionCounterCmd extends BaseAsyncCmd {
Review Comment:
===> DeleteAutoScaleCounterCmd ?
##
api/src/main/java/org/apache/cloudstack/api/command/admin/autoscale/CreateCounterForAutoScaleConditionCmd.java:
##
@@ -34,7 +34,7 @@
@APICommand(name = "createCounter", description = "Adds metric counter for VM
auto scaling", responseObject = CounterResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
-public class CreateCounterCmd extends BaseAsyncCreateCmd {
+public class CreateCounterForAutoScaleConditionCmd extends BaseAsyncCreateCmd {
Review Comment:
===> CreateAutoScaleCounterCmd ?
##
api/src/main/java/org/apache/cloudstack/api/command/user/autoscale/ListAutoScaleConditionCountersCmd.java:
##
@@ -33,7 +33,7 @@
@APICommand(name = "listCounters", description = "List the counters for VM
auto scaling", responseObject = CounterResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
-public class ListCountersCmd extends BaseListCmd {
+public class ListAutoScaleConditionCountersCmd extends BaseListCmd {
Review Comment:
==> ListAutoScaleCountersCmd ?
##
server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java:
##
@@ -1527,7 +1527,7 @@ public Condition createCondition(CreateConditionCmd cmd) {
}
@Override
-public List listCounters(ListCountersCmd cmd) {
Review Comment:
will you rename Counter and Condition class too ?
can do later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
erikbocks commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3451067896 @DaanHoogland @weizhouapache, I renamed the other classes names, could you check them now? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
github-actions[bot] commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3451020047 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
codecov[bot] commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3390184441 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/11819?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 3.59%. Comparing base ([`b0c7719`](https://app.codecov.io/gh/apache/cloudstack/commit/b0c77190066fd74336ea831ccf9e5427e793ba7f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a8645f3`](https://app.codecov.io/gh/apache/cloudstack/commit/a8645f3ccd0ac200b7e6bf7dc5551512867ec31f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 49 commits behind head on main. > :exclamation: There is a different number of reports uploaded between BASE (b0c7719) and HEAD (a8645f3). Click for more details. > > HEAD has 1 upload less than BASE > >| Flag | BASE (b0c7719) | HEAD (a8645f3) | >|--|--|--| >|unittests|1|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #11819 +/- ## = - Coverage 17.38%3.59% -13.79% = Files 5891 443 -5448 Lines52635637400 -488956 Branches 64270 6868-57402 = - Hits 91526 1346-90180 + Misses 42448835891 -388597 + Partials 10342 163-10179 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/11819/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/11819/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.59% <ø> (-0.02%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/11819/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/11819?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
erikbocks commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3406407750 I agree with the `CreateAutoScaleCondition` class name, and I will try to work in this PR later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
weizhouapache commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3406431945 > I agree with the `CreateAutoScaleCondition` class name, and I will try to work in this PR later. can you please also change the other classes for AS condition ? Update, Delete, List ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3396077585 I like @weizhouapache ’s name best, `CreateAutoScaleCondition`. @erikbocks , what do you think? cc @shwstppr -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
weizhouapache commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3390439828 > @erikbocks , sensible change but I would leave out the “Vm” bit from the new name. “CreateConditionForAutoScalingCmd” seems more appropriate as the VM is not scaled itself but the service is scaled by adding more (or less) VMs.What do you think? cc @weizhouapache @shwstppr . maybe use `CreateAutoScaleCondition` which consistent with other files: createAutoScalePolicy createAutoScaleVmProfile createAutoScaleVmGroup -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
shwstppr commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3390685439 @DaanHoogland I don't have a strong preference either way. I don't even mind class remain CreateCondition as personally I find it easier during troubleshooting to search the class with API name. @weizhouapache suggestion also makes sense as it keeps naming concise. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd [cloudstack]
DaanHoogland commented on PR #11819: URL: https://github.com/apache/cloudstack/pull/11819#issuecomment-3390168104 @erikbocks , sensible change but I would leave out the “Vm” bit from the new name. “CreateConditionForAutoScalingCmd” seems more appropriate as the VM is not scaled itself but the service is scaled by adding more (or less) VMs.What do you think? cc @weizhouapache @shwstppr . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
