Adam Litke has posted comments on this change.

Change subject: Detect optional parameter syntax errors in JSON schema
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

This just requires a minor change to the wording of the error messages.  Also, 
when it is rebased, the current schema should be checked for errors and this 
patch should include the fixes to bring the schema into compliance.

....................................................
File vdsm_api/process-schema.py
Line 165:                         raise ValueError(
Line 166:                             'Description of %s optional parameter 
error:\n'
Line 167:                             '\tThe description of optional parameter 
"%s" '
Line 168:                             'should start with "#optional"' %
Line 169:                             (symbol['name'], name))
The error messages above always assume that if the optional designation appears 
only in the documentation comment or the 'data' structure field (but not both) 
that the fix is to add the missing designation.  To properly fix these errors 
we must be careful to check whether the parameter is really optional or not.  
Therefore, for either case above I would recommend a more general error message:

Symbol '%s' optional parameter declaration error.  The description of parameter 
'%s' is tagged '#optional' but not defined as optional.

Symbol '%s' optional parameter declaration error.  The parameter '%s' is 
defined as optional but the description is missing the '#optional' tag.
Line 170:             else:
Line 171:                 # Just append it to the last one we added
Line 172:                 symbol[mode][last_arg] += ' ' + line
Line 173:         elif mode == 'info_return':


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f92459ee8787780a54a510b6f8cc074fb2a235
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Better Saggi <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[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