brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geomacy <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #454: LocalEntityManager: make fields private
Date Wed, 23 Nov 2016 13:25:00 GMT
Github user geomacy commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/454#discussion_r89313611
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
---
    @@ -742,7 +742,7 @@ private boolean unmanageNonRecursive(Entity e) {
                 if (e instanceof Group) {
                     Collection<Entity> members = ((Group)e).getMembers();
                     for (Entity member : members) {
    -                    if (!Entities.isNoLongerManaged(member)) member.removeGroup((Group)e);
    +                    if (!Entities.isNoLongerManaged(member)) ((EntityInternal)member).groups().remove((Group)e);
    --- End diff --
    
    `removeGroup` is deprecated, but it's just defined as 
    ```java
       public void removeGroup(Group group) {
            groups().remove(group);
        }
    ```
    so I don't see how this change is changing anything really? Is it not just doing the same
thing? The comments on removeGroup are slightly ambiguous but seem to suggest that the preferred
thing to be doing here is calling `Group#removeMember`, which will update the group size sensors
and so on.  Same thought applies to the changes to `add` below.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message