Alon Bar-Lev has posted comments on this change.

Change subject: core: rewrite buildSequence using toposort
......................................................................


Patch Set 3:

(3 comments)

did not actually review... but we cannot break backward compatibility... and I 
still do not understand the actual problem we are trying to solve compared to 
the extra complexity added.

http://gerrit.ovirt.org/#/c/28775/3/src/otopi/context.py
File src/otopi/context.py:

Line 170: 
Line 171: 
Line 172:     # Based on https://pypi.python.org/pypi/toposort/1.0
Line 173:     def _toposort(self, data):
Line 174:         from functools import reduce as _reduce
please do not use python3 obsoleted functions, if there is no reduce in python3 
use lambda or whatever to solve it properly in python3
Line 175:         """Dependencies are expressed as a dictionary whose keys are 
items
Line 176:         and whose values are a set of dependent items. Output is a 
list of
Line 177:         sets in topological order. The first set consists of items 
with no
Line 178:         dependences, each subsequent set consists of items that 
depend upon


Line 200:                 break
Line 201:             yield ordered
Line 202:             data = {item: (dep - ordered)
Line 203:                     for item, dep in data.items()
Line 204:                         if item not in ordered}
indentation is wrong, not sure how this works and the above _reduce is required
Line 205:         if len(data) != 0:
Line 206:             raise ValueError('Cyclic dependencies exist among these 
items: {}'.format(', '.join(repr(x) for x in data.items())))
Line 207: 
Line 208:     @property


Line 202:             data = {item: (dep - ordered)
Line 203:                     for item, dep in data.items()
Line 204:                         if item not in ordered}
Line 205:         if len(data) != 0:
Line 206:             raise ValueError('Cyclic dependencies exist among these 
items: {}'.format(', '.join(repr(x) for x in data.items())))
cannot be that pass pep8, please use gettext
Line 207: 
Line 208:     @property
Line 209:     def environment(self):
Line 210:         """Environment."""


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a7e9a26ac68543331dc869ab151883ac8bd5b30
Gerrit-PatchSet: 3
Gerrit-Project: otopi
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <d...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Lev Veyde <lve...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to