2008/6/12 <[EMAIL PROTECTED]>: >> > http://cr.opensolaris.org/~johansen/pkg-timeout2/ >> >> filelist.py: >> >> - line 121: "receive" or "retrieve"? > > Thanks, I meant retrieve, but obviously typed something else. > >> - line 123: "This is a private method which performs the majority of the >> work for add_content()." (Add ", which serves as a wrapper to handle >> exceptions and retrieval from cache", if you really want.) > > Ok. > >> - line 168: I'd ditch this blank line. > > Ok. > >> image.py: >> >> - line 455: I don't mean to make this silly, but perhaps this should be >> _fetch_manifest_with_retries(), and what's now _download_manifest() >> should be _fetch_manifest()? I don't think that the (English) >> difference between "fetch" and "download" really tells us what the >> difference between the two methods is. > > Ok. > >> - line 1222: "Clean"
http://cr.opensolaris.org/~johansen/pkg-timeout2/src/modules/client/image.py.wdiff.html ========== 1191 + if len(fields) < 4: A comment here explaining why you there is a varying number of fields would be nice. http://cr.opensolaris.org/~johansen/pkg-timeout2/src/modules/client/pkgplan.py.html ========== 234 if dest: 235 dest.preinstall(self, src) ... 239 if dest and dest.needsdata(src): 240 flist.add_action(dest) Would that be better as: if dest: dest.preinstall(self, src) if dest.needsdata(src): flist.add_action(dest) http://cr.opensolaris.org/~johansen/pkg-timeout2/src/client.py.html ========== 1372 misc.MAX_TIMEOUT_COUNT = int(os.environ.get("PKG_TIMEOUT_MAX", "3")) Doesn't the "hard-coded" 3 here make specifying a default value in misc.py not as useful? Cheers, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
