[pkg-discuss] Code review 1338

[Brock Pytlik]

Bart Smaalders wrote:
> Brock Pytlik wrote:
>   
>> When you have a chance please take a look at
>> http://cr.opensolaris.org/~bpytlik/ips-fix-1338/
>>
>>
>> It adds test cases for bug 1338 and also changes the error message 
>> reported to the user to something hopefully more useful.
>>
>>     
>
> Looks good.. I see that error is superfluous:
>
> : barts at cyber[8]; python
> Python 2.4.4 (#1, Feb 25 2008, 04:14:47) [C] on sunos5
> Type "help", "copyright", "credits" or "license" for more information.
>  >>> a = ""
>  >>> if a:
> ...   print "a"
> ... else:
> ...   print "not a"
> ...
> not a
>  >>>
>
>   
Ok, I'll change that

> so you can just do the same test w/ outstring as w/ error.
>
> You may wish to consider your wording:
>
>        351 +                                outstring += "Installing %s 
> causes:\n\t%s\n" % \
>        352 +                                    (f.get_name(), e)
>
> as there was no attempt to install; perhaps "Cannot install" would be 
> better?
>
>   
Here I'm confused why you say there was no attempt to install. From the 
user's perspective, attempting to install a package is what causes this 
error. If I changed "Installing" to "Attempting to install" would that 
be better?

Thanks for your feedback,
Brock
> Thanks for adding the test case - we don't have enough of those.
>
> - Bart
>
>
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.opensolaris.org/pipermail/pkg-discuss/attachments/20080501/d9ae776f/attachment.html>