On Mon, May 12, 2008 at 6:09 PM, Brock Pytlik <[EMAIL PROTECTED]> wrote:
> http://cr.opensolaris.org/~bpytlik/ips-fix-1081-and-777/
>
>  Is a fix for
>  1081: pkg install in a not-yet-created image now gives a traceback
>  777: Image.find_root thinks a repository is an image


http://cr.opensolaris.org/~bpytlik/ips-fix-1081-and-777/src/modules/client/image.py.wdiff.html

      129 +                self.image_subdirs = ["catalog", "file",
"pkg", "index"]

Wouldn't this be better moved up above __init__ since it isn't changed
by any-code (class attribute instead of instance attribute)?

      152 +                            check_subdirs(d,img_user_prefix)):
      164 +                              check_subdirs(d,img_root_prefix)):
Missing space after ','.

      208 +                        if not os.path.isdir(self.imgdir + "/" + sd):
      209 +                                os.makedirs(self.imgdir + "/" + sd)

Shouldn't these use os.path.join?

http://cr.opensolaris.org/~bpytlik/ips-fix-1081-and-777/src/tests/cli/t_pkg_install_corrupt_image.py.html

  22

I think there's supposed to be a '#' at the beginning of the line for
the CDDL header block.

  63                 Create a good image at $basedir/image. Create a corrupted

The convention I've seen so far is to put two spaces after a period in
docstrings. This is in several places, but happened to be the first I
saw.

  66                 and set PKG_IMAGE to be that directory."""

No space at the end, but you had a space at the beginning of the
docstring. In addition, I'm not sure what our accepted convention is
for docstrings when it comes to leading spaces. I've seen 4 spaces
placed before a continuation, and I've also seen no spaces as you've
done. Don't know which is right. This is in a few other places too.

 241                 self.corrupt_image_create(durl, set([]),["var/pkg"])
 441                 self.corrupt_image_create(durl,
set([]),[".org.opensolaris,pkg"])

Missing space after last ','.

http://cr.opensolaris.org/~bpytlik/ips-fix-1081-and-777/src/tests/cli/testutils.py.udiff.html


      534 +                if(not(self.backUpIP)):

The if statements in this file are missing a space before the first parentheses.

      543 +

Extra line?

      579 +
os.remove(os.path.join(tmpDir,"cfg_cache"))
      581 +
shutil.rmtree(os.path.join(tmpDir,"file"))
      583 +
shutil.rmtree(os.path.join(tmpDir,"pkg"))
      585 +
shutil.rmtree(os.path.join(tmpDir,"index"))
      587 +                self.img_path = os.path.join(self.img_path,"act_img")

Missing space after ','.

Otherwise, looks ok to me.

-- 
Shawn Walker

"To err is human -- and to blame it on a computer is even more so." -
Robert Orben
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to