Vinzenz Feenstra has posted comments on this change.

Change subject: java_biniding: Simplify bindings by using object mappers
......................................................................


Patch Set 4: Code-Review-1

(6 comments)

I guess it's be good to decide if you use tabs or no tabs in the java source 
files.
It's quite mixed in a lot of files.

Additionally why are you removing the Host.setMOMPolicyParameters method from 
the schema?

....................................................
Commit Message
Line 3: AuthorDate: 2013-07-18 13:20:18 +0300
Line 4: Commit:     Saggi Mizrahi <[email protected]>
Line 5: CommitDate: 2013-08-07 15:06:31 +0300
Line 6: 
Line 7: java_biniding: Simplify bindings by using object mappers
java_binding
Line 8: 
Line 9: Using object mappers instead of object wrappers simplifies things
Line 10: considerably.
Line 11: 


....................................................
File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/RequestBuilder.java
Line 12:     private final ObjectNode _parameters;
Line 13:     private final String _methodName;
Line 14: 
Line 15:        static {
Line 16:                _objMapper = new ObjectMapper();
Are those tabs wanted? everywhere else it's replaced now
Line 17:        }
Line 18: 
Line 19:     public RequestBuilder(String methodName) {
Line 20:         _id = null;


....................................................
File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/VDSMObjectMapper.java
Line 100:         } catch (IllegalAccessException | InvocationTargetException | 
NoSuchMethodException e) {
Line 101:             throw new RuntimeException(e);
Line 102:         }
Line 103:     }
Line 104:       @SuppressWarnings("unchecked")
tab & no tab mix in this file
Line 105:     private Class<?> _getConcreteClass(ObjectNode node, Class<?> cls) 
{
Line 106:         Field stmapField = null;
Line 107:         Field stNameField = null;
Line 108:         try {


....................................................
File client/java/vdsm-api/src/main/java/org/ovirt/vdsm/VDSMObjectSerializer.java
Line 19:        public void serialize(VDSMObject t, JsonGenerator jg, 
SerializerProvider sp) throws IOException {
Line 20:                Class cls = t.getClass();
Line 21:                jg.writeStartObject();
Line 22:                for (Method method : cls.getMethods()) {
Line 23:                        
empty line with tabs
Line 24:                        if (method.getParameterTypes().length != 0) {
Line 25:                                continue;
Line 26:                        }
Line 27: 


Line 49:                }
Line 50: 
Line 51:                jg.writeEndObject();
Line 52:        }
Line 53:        
same here


....................................................
File vdsm_api/vdsmapi-schema.json
Line 466
Line 467
Line 468
Line 469
Line 470
Why are you deleting this? This does not seem to be scope of this patchset...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida303667cbba365bf14cc10147bbf507e3b730d6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to