Shawn Walker wrote: > Brock Pytlik wrote: >> Resynced again: >> http://cr.opensolaris.org/~bpytlik/ips-4260-v3/ >> >> I'd like this to make 2008.11, so if anyone has a spare cycle or two >> to look at this, I'd really appreciate it. > > doc/api_versions.txt: > It would be nice if you could rename this client_api_versions.txt > since there will eventually be a server api as well. Fine > > client/api.py: > line 698: This seems unnecessary given that > ca.parse_category_info() already checks for this. I would prefer to keep it there so that other people don't have to wonder what happens in generator functions if an empty list is returned. The code as written makes it clearer that category, and only category info is included in the list. Also, suppose parse_category_info changed to return None, or False, instead of [], I think that could lead to confusing error messages. I doubt that the extra function call to has_category_info imposes enough overhead to outweigh the (imo) clearer code. > > line 843-845: this can be simplified to: > self.category_info_list = category_info_list or [] I'd rather not do this, though if others think I should I will. I find this kind of construction hard to follow, and prefer not to use it without a good reason. > > cli/t_api_info.py: > For the paranoid person in me, can you add a test > info.classification that is more than just one or two categories deep > just to ensure we handle the n-deep cases? > Sure.
Brock > Otherwise, looks fine. > > Cheers, _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
