On 06/ 1/12 10:05 AM, Shawn Walker wrote:
On 05/29/12 21:40, Tim Foster wrote:
https://cr.opensolaris.org/action/browse/pkg/timf/pkgdep-smf-req-any
Thanks for taking a look,
src/modules/flavor/base.py
line 88: s/raise/raised/
line 97: If this error will be seen by the user, it should be more
descriptive.
line 238: s/non-None/not None/
Fixed those - the error won't be seen by a user, since it indicates a
programming error error in creating the PublishingDependency object (I
considered an assert, but thought an exception would be better)
src/modules/flavor/smf_manifest.py:
line 284: this should be moved inside the exception case, as its
pointless to assign a value and then immediately reassign on
line 286
I don't quite follow? If I do that, 'manifests' won't get defined when
the exception is thrown. It's analogous to:
1 #!/usr/bin/python2.6
2 import random
3
4 def value():
5 if random.choice([True, False]):
6 raise Exception("something went wrong")
7 return ["value"]
8
9 try:
10 manifests = value()
11 except:
12 print "not to worry"
13 print "manifests is %s " % len(manifests)
running it when the exception isn't thrown works fine, but when we throw
an exception, we have:
$ ./test.py
not to worry
Traceback (most recent call last):
File "./t.py", line 13, in <module>
print "manifests is %s " % len(manifests)
NameError: name 'manifests' is not defined
line 302: the comment seems confusing given line 309
Good point, I'll remove the comment - here we're only gathering
information to determine whether we've got an instance or a service, we
check which it is at line 309.
I mainly did a sanity check; someone more familiar with the dependency
code should look over this too.
No worries, thanks again for the review,
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
