Allon Mureinik has posted comments on this change.

Change subject: findbugs: Make inner classes static where possible
......................................................................


Patch Set 1:

(2 comments)

> A workaround would be to make all classes annotated with 
> @GenEvent top-level (not inner), the main reason is 
> convenience (small inner class vs. separate top-level 
> class). In future, we can make all classes annotated with 
> @GenEvent top-level.
Agreed, but I wouldn't do it in the scope of this patch - let's leave that for 
some future refactoring.


> BTW, it would be really nice if findbugs allowed 
> suppressing "bugs" (like SIC_INNER_SHOULD_BE_STATIC) based 
> on annotation presence, I guess this is not supported yet 
> in findbugs.
AFAIK, there is no such capability.

http://gerrit.ovirt.org/#/c/26227/1/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/smtp/Smtp.java:

Line 125:                             MAIL_USER));
Line 126:         }
Line 127:         if (StringUtils.isNotEmpty(emailPassword)) {
Line 128:             session = Session.getDefaultInstance(mailSessionProps,
Line 129:                     new EmailAuthenticator(emailUser, emailPassword));
> Non related change
auto-formatting, will fix.
Line 130:         } else {
Line 131:             session = Session.getInstance(mailSessionProps);
Line 132:         }
Line 133:     }


http://gerrit.ovirt.org/#/c/26227/1/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java
File 
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/resources/VmTable.java:

Line 462:     }
Line 463: 
Line 464: }
Line 465: 
Line 466: class StyledCompositeCell<T> extends CompositeCell<T> {
> Alissa, I think that putting "static" here would violate Java language spec
Exactly - it's a top level class.
(ugly as heck having it in this file, BTW. I smell my next patch coming up... 
`-))
Line 467: 
Line 468:     private final List<HasCell<T, ?>> hasCells;
Line 469:     private final StyledProvider<T> styleProvider;
Line 470: 


-- 
To view, visit http://gerrit.ovirt.org/26227
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbceb1baad596211580c1289f49f7d12e33a8ecb
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to