Hi Collin,
> Patch 0005 changes a list to a set. Previously, it checked if each
> file was a member of the list before appending it. We can just let
> Python do this for us. I've also sorted before returning to make it
> behave like gnulib-tool.sh.
Typically, when one changes a function, one should ask oneself
"should the doc string be updated?"
So. Should the doc string include a hint
'''The resulting list is sorted.''' ?
No, that would be an over-specification of this function's behaviour.
The callers of this functions will sort themselves.
So. Should the doc string say
'''The sorting order order on the resulting list is unspecified.
The caller needs to sort according to their criteria.'''
This is better. But wait: If a return value is a list with unspecified
sorting order, and doesn't contain duplicates, then list is the wrong
data type. The function should return a set then.
=> Not applied.
> Patch 0006 turns the avoided modules in GLModuleTable into a set
> instead of a list. Since we check every module for membership in the
> avoided modules, it makes more sense to use a set. The avoided modules
> emitted in the actioncmd and gnulib-comp.m4 are stored in GLConfig, so
> doing this doesn't break anything.
This patch goes into a right direction. But when the field contains a set,
the accessors getAvoids and setAvoids should IMO also return a set and
accept a set, respectively.
=> Not applied.
> Patch 0007 uses defaultdict() instead of dict() for module
> dependencies. This has been around for a long time and deals with the
> explicit initialization case for you:
It's a nice Python class. But the use of defaultdict(list) makes only
the two lines
if str(module) not in self.dependers:
self.dependers[str(module)] = []
redundant. It does not make the line
if str(parent) not in self.dependers[str(module)]:
redundant. self.dependers[str(module)] should not contain duplicates.
=> Not applied.
Bruno
