[GitHub] [cloudstack] SadiJr commented on a change in pull request #5043: Allow updating the storage/host tags of service offerings

[GitBox <gi...@apache.org>]

SadiJr commented on a change in pull request #5043:
URL: https://github.com/apache/cloudstack/pull/5043#discussion_r648551221



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -3332,14 +3312,14 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) {
     }
 
     /**
-     * Check the tags parameters to the diskOffering
+     * Check the tags parameters to the disk/service offering
      * <ul>
      *     <li>If tags is null, do nothing and return.</li>
-     *     <li>If tags is not null, set tag to the diskOffering.</li>
-     *     <li>If tags is an blank string, set null on diskOffering tag.</li>
+     *     <li>If tags is not null, set tag to the disk/service offering.</li>
+     *     <li>If tags is an blank string, set null on disk/service offering tag.</li>
      * </ul>
      */
-    protected void updateDiskOfferingTagsIfIsNotNull(String tags, DiskOfferingVO diskOffering) {
+    protected void updateOfferingTagsIfIsNotNull(String tags, DiskOfferingVO diskOffering) {
         if (tags == null) { return; }
         if (StringUtils.isNotBlank(tags)) {
             diskOffering.setTags(tags);

Review comment:
       I think it would be interesting to use the same logic as the hostTags conditional here, allowing you to remove existing tags.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org