Vojtech Szocs has posted comments on this change.

Change subject: core: Add no-argument constructor check
......................................................................


Patch Set 2:

(2 comments)

> Looking at 
> build-tools-root/checkstyles/src/main/resources/checkstyle-suppressions.xml I 
> don't understand why I need both that and the annotation. Wouldn't it be 
> simpler (and less intrusive) to just list all the problematic files there?

Excellent point. At first I went with @Disable annotation, then added 
checkstyle-suppressions.xml to filter out Test classes, and ultimately forgot 
to realize that checkstyle-suppressions.xml can replace such annotation 
completely, simplifying the overall solution.

I agree with Allon - I'll add classes marked with @Disable into 
checkstyle-suppressions.xml and get rid of @Disable annotation and related 
changes.

> [last time we discussed annotations, I was under the impression that the 
> <surpress> concept was out the window for some reason]

My bad, I wasn't aware of checkstyle suppression feature until recently.

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/LocationInfo.java
Line 6:     public LocationInfo(ConnectionMethod connectionMethod) {
Line 7:         this.connectionMethod = connectionMethod;
Line 8:     }
Line 9: 
Line 10:     public LocationInfo() {
Because LocationInfo is used in certain Parameters classes:
* DownloadImageVDSCommandParameters
* UploadImageVDSCommandParameters

Now I see that these Parameters classes (and related commands) are used only 
internally on backend.

I agree that this class and HttpLocationInfo can be excluded from no-arg check.
Line 11:     }
Line 12: 
Line 13:     public ConnectionMethod getConnectionMethod() {
Line 14:         return connectionMethod;


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ListUtils.java
Line 37:      * Creates a filteres list with the elements evaluated to 
<strong>true</strong>.
Line 38:      *
Line 39:      * @param <T>
Line 40:      */
Line 41:     @NoArgConstructorCheck.Disable
Agreed, any improvements should be done preferably in a separate patch.
Line 42:     public static class PredicateFilter<T> implements Filter<T> {
Line 43: 
Line 44:         public PredicateFilter(Predicate<T> predicate) {
Line 45:             super();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4f9a00d6289374b433ed4419552420a3337da50
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to