Piotr Kliczewski has posted comments on this change.

Change subject: bridge: data verification
......................................................................


Patch Set 29:

(7 comments)

https://gerrit.ovirt.org/#/c/53919/29/lib/api/schemaapi.py
File lib/api/schemaapi.py:

PS29, Line 32: primitive_types
> ALL CAPS please
Done


PS29, Line 170: arg
> arg->value
Done


PS29, Line 180: .keys()
> needless
Done


Line 211:                 self._verify_complex_type(t.get('type'), t, arg, name,
Line 212:                                           class_name, method_name)
Line 213: 
Line 214:     def _verify_complex_type(self, t_type, t, arg, name, class_name,
Line 215:                              method_name):
> I could use a docstring here.
Done
Line 216:         if t_type == 'alias':
Line 217:             # if alias we need to check sourcetype
Line 218:             self._check_primitive_type(t.get('sourcetype'), arg, name)
Line 219:         elif t_type == 'map':


PS29, Line 221: iteritems
> use six.iteritems; new code should be python3 compliant.
Done


Line 245:                                                        t.get('name'),
Line 246:                                                        class_name,
Line 247:                                                        method_name))
Line 248:         else:
Line 249:             # if object we need to check whether all the properties
> this long branch begs to be broken to a helper function
Done
Line 250:             # match values provided
Line 251:             props = t.get('properties')
Line 252:             prop_names = [prop.get('name') for prop in props]
Line 253:             # check if there are any extra prarameters


https://gerrit.ovirt.org/#/c/53919/29/tests/Makefile.am
File tests/Makefile.am:

Line 124:       resourceManagerTests.py \
Line 125:       responseTests.py \
Line 126:       samplingTests.py \
Line 127:       scheduleTests.py \
Line 128:       schemaapi_test.py \
> please try to add to py3_test modules
Will check.
Line 129:       schemaTests.py \
Line 130:       schemaValidationTest.py \
Line 131:       sdm_indirection_tests.py \
Line 132:       securableTests.py \


-- 
To view, visit https://gerrit.ovirt.org/53919
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id24a5e078fa92e4129d37a47593c7a167e78712e
Gerrit-PatchSet: 29
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to