Michael Pasternak has posted comments on this change.

Change subject: codegen: Use relative paths
......................................................................


Patch Set 2:

(3 comments)

new arguments usually got added to the end of method so it won't mislead the 
developers,

please consider following this pattern across your change

http://gerrit.ovirt.org/#/c/23368/2/src/codegen/rsdl/rsdlcodegen.py
File src/codegen/rsdl/rsdlcodegen.py:

Line 85: 
Line 86:         # Get all the links from the RSDL document an make them 
relative:
Line 87:         links = proxy.request('GET', '?rsdl').links.link
Line 88:         for link in links:
Line 89:             link.href = link.href.replace(prefix, "")
this change is too risky as 'prefix' may appear in future in path not in 
position [0] only, i'd stay with old splitted_url.pop(0)
Line 90: 
Line 91:         for link in links:
Line 92: 
Line 93:             response_type = None


http://gerrit.ovirt.org/#/c/23368/2/src/ovirtsdk/infrastructure/proxy.py
File src/ovirtsdk/infrastructure/proxy.py:

Line 28:         @param prefix: the prefix common to all requests, usually 
/ovirt-engine/api
Line 29:         @param connections_pool: connections pool
Line 30:         @param persistent_auth: persistent authentication flag 
(default True)
Line 31:         '''
Line 32:         self._prefix = prefix
i'd make it private
Line 33:         self.__connections_pool = connections_pool
Line 34:         self._persistent_auth = persistent_auth
Line 35: 
Line 36:         # In order to create the cookies adapter we need to extract 
from the


Line 28:         @param prefix: the prefix common to all requests, usually 
/ovirt-engine/api
Line 29:         @param connections_pool: connections pool
Line 30:         @param persistent_auth: persistent authentication flag 
(default True)
Line 31:         '''
Line 32:         self._prefix = prefix
i'd make it private
Line 33:         self.__connections_pool = connections_pool
Line 34:         self._persistent_auth = persistent_auth
Line 35: 
Line 36:         # In order to create the cookies adapter we need to extract 
from the


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e39510541e34349fce5a8ad4a643cf65784bccc
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine-sdk
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to