On 08/24/11 16:39, Shawn Walker wrote:
On 08/24/11 15:57, Brock Pytlik wrote:
On 08/23/11 18:33, Shawn Walker wrote:
Greetings,
The following webrev contains fixes for the following issue:
17252 pkg.client.api should provide history management interface
webrev:
https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-history-1/webrev/
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
client.py:
is there any reason that img (lines 5754, 5761, 5939), continues to need
to be global? or even to exist at all?
Sadly, the property*, fix, verify, and facet subcommands still use it :-(
5545: does defining gen_entries simply let you avoid indenting 5554-5646
(roughly)? If so, that's fine, but it might be worth a comment, and if
it serves another purpose, can you call that out please?
Yes, basically. I'll add a comment.
I thought it was preferable over attempting to squeeze all of the
logic below into 80 columns. The other option would be to simply move
that entire block into a separate function.
Which would you prefer?
What you have is fine, but whenever I see code like that, I always spend
time verifying that there isn't something magical going on there. The
comment simply speeds up my willingness to skip over it.
api.py:
gen_history could use a docstring
Yes, I missed that. I've already added in my local workspace.
1880: the number "20" here seems a bit magical, perhaps the comment on
line 1886 could be moved up?
1896 I assume, but yes. Sorry, this was all pretty much just moved
from client.py.
Hmm, no, I think I meant 1886? The comment that starts:
# Ranges are 19 chars of timestamp,
Brock
Thanks,
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
