Shawn Walker wrote:
Brock Pytlik wrote:
client.py:
356: Is there a reason to use the string "known" here instead of a
constant?
No constant exists, and it seemed silly to create a constant for a
single case.
I guess:
state = "known"
followed two lines later by:
state = img.PKG_STATE_INSTALLED
looks strange to me.
Also, does img.PKG_STATE_INSTALLED even exist anymore? It kinda looks
like it's been moved/removed.
2276: Is there a reason to make this change?
More efficient as it avoids assignment to a variable that is
immediately discarded? That and pylint complained ;)
I guess I find the former code easier to read as it suggests that
something of a known, fixed, length is being returned, as opposed just
grabbing the first of whatever is in the answer.
publisher.py:
946: why do the import here?
Avoids the overhead of importing the module until it's needed.
We should probably decide whether we want to start doing this or not. I
think, so far, we've gone with a "import everything at the top"
approach. I brought this up to dp last week and he believed it wouldn't
make much of a difference in terms of performance. (I haven't had the
time to do the performance testing myself.) I'd suggest that if we're
going in this direction, we make a decision to do it as a team, and look
to make those changes as we touch bits of code. Otherwise, I don't see a
reason that this needs special treatment. If the module is somehow
special, please explain why :)
manifest.py:
481-482: why not just compare signatures with self.signatures?
Because that would be a self-fulfilling prophecy :)
The signatures are generated based on the current contents, so
validate would always be true.
This allows an external caller with the signature data they have for
the manifest to validate easily against what the manifest thinks its
signatures are.
Sorry, let me be more explicit why is this code:
481 + old_signatures = self.signatures
482 + if signatures != old_signatures:
483 + raise
api_errors.BadManifestSignatures(self.fmri)
Different from this code:
482 + if signatures != selfg.signatures:
483 + raise
api_errors.BadManifestSignatures(self.fmri)
?
server/catalog:
We could probably just remove the checks for .pag and .dir? Those
haven't existed since the 2008.05 release.
1151: what are the possible classes that could be coming in here?
It's marked as a classmethod now so it can access __parse_entry.
Ok, I'll try again. Why is it a static method instead of a class method?
Ie, what other classes than the current one do you anticipate filling cls?
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
