Thanks Ethan!

Tim

Ethan Quach wrote:
> Tim,
> 
> The changes look fine to me.
> 
> 
> -ethan
> 
> 
> Tim Knitter wrote:
>> Ethan,
>>
>>  
>>> Tim,
>>>
>>> Fix lines 30 and 75; they cause line overflows.
>>>     
>>
>> So a character in the 67th column creates an overflow? How is that 
>> determined?
>>
>>  
>>> Can you remove the change at line 4.  I think having this in
>>> one place in the DESCRIPTION section is sufficient.
>>>
>>>     
>>
>> done.
>>
>> Thanks
>> Tim
>>
>>  
>>> thanks,
>>> -ethan
>>>
>>>
>>> Tim Knitter wrote:
>>>    
>>>> Ethan,
>>>>
>>>>  
>>>>      
>>>>> Tim,
>>>>>
>>>>> One more comment in beadm.1m.txt
>>>>>
>>>>> 41 - Do we really need this line here?   Its not a special individual
>>>>>             
>>>> No. It's nuked.
>>>>
>>>> Thanks
>>>> Tim
>>>>
>>>>  
>>>>      
>>>>> operation, so why are we highlighting it?  When 1833 gets fixed and
>>>>> we have something like a "beadm description ... "  or whatever
>>>>> subcommand, that would be something we'd add to this list.
>>>>>
>>>>>
>>>>> thanks,
>>>>> -ethan
>>>>>
>>>>>
>>>>> Tim Knitter wrote:
>>>>>           
>>>>>> Ethan Quach wrote:
>>>>>>  
>>>>>>               
>>>>>>> Ethan Quach wrote:
>>>>>>>                      
>>>>>>>> Tim Knitter wrote:
>>>>>>>>  
>>>>>>>>                            
>>>>>>>>> Ethan Quach wrote:
>>>>>>>>>                                     
>>>>>>>>>> Tim,
>>>>>>>>>>
>>>>>>>>>> The code changes look fine.  My only comment is on the man page.
>>>>>>>>>> Ideally, I'd rather us not mention the use of 'description' as 
>>>>>>>>>> being used
>>>>>>>>>> as the title in the grub menu at all if possible.  ~But~ if 
>>>>>>>>>> this *is* something
>>>>>>>>>> we do want to publicize, then it should be stated as a side 
>>>>>>>>>> note rather than
>>>>>>>>>> the only use of 'description'.  i.e. something like:
>>>>>>>>>>
>>>>>>>>>>    -d description        Create a new BE with a description
>>>>>>>>>>                          associated with it.  This description
>>>>>>>>>>                          is also used as the title for this BE's
>>>>>>>>>>                          entry in the GRUB menu.
>>>>>>>>>>
>>>>>>>>>>                                                 
>>>>>>>>> I like this better than the example I passed back to Barbara 
>>>>>>>>> earlier with the addition of the last sentence I added.
>>>>>>>>>
>>>>>>>>> "If a description is not supplied, beName will be used as the 
>>>>>>>>> entry in the GRUB menu."
>>>>>>>>>
>>>>>>>>>                                         
>>>>>>>> Hmm, could we instead move the last two sentences out into the main
>>>>>>>> beadm create paragraph above?
>>>>>>>>                                 
>>>>>>> .. the reason for this request is that with this additional 
>>>>>>> sentence, seems like
>>>>>>> we're starting to describe behavior outside of what's just 
>>>>>>> inherent for the option.
>>>>>>>
>>>>>>>                         
>>>>>> Agreed. Webrev updated.
>>>>>>
>>>>>> Thanks
>>>>>> Tim
>>>>>>
>>>>>>  
>>>>>>               
>>>>>>> -ethan
>>>>>>>
>>>>>>>                      
>>>>>>>> thanks,
>>>>>>>> -ethan
>>>>>>>>
>>>>>>>>
>>>>>>>>  
>>>>>>>>                            
>>>>>>>>>                                     
>>>>>>>>>> Also, could you add an Example to show how to quote the 
>>>>>>>>>> description
>>>>>>>>>> text on the command line.
>>>>>>>>>>
>>>>>>>>>>                                                 
>>>>>>>>> Done.
>>>>>>>>> See updated webrev.
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Tim
>>>>>>>>>
>>>>>>>>>                                     
>>>>>>>>>> thanks,
>>>>>>>>>> -ethan
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Tim Knitter wrote:
>>>>>>>>>>                                             
>>>>>>>>>>> In order to test the functionality that this code exposes, 
>>>>>>>>>>> beadm has also been
>>>>>>>>>>> modified to make use of the new interface in libbe.c and is 
>>>>>>>>>>> being addressed by
>>>>>>>>>>> this bug. A [-d description] opt & arg is beaing added to the 
>>>>>>>>>>> beadm usage for create as follows:
>>>>>>>>>>>
>>>>>>>>>>> beadm create [-a] [-d description] [-e non-activeBeName | 
>>>>>>>>>>> beName at snapshot]
>>>>>>>>>>>     [-o property=value] ... [-p zpool] beName
>>>>>>>>>>>
>>>>>>>>>>> Please see the updated webrev that includes the changes to 
>>>>>>>>>>> make the above happen.
>>>>>>>>>>>
>>>>>>>>>>> These changes have been successfully tested on bld99.
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>> Tim
>>>>>>>>>>>  
>>>>>>>>>>> Tim Knitter wrote:
>>>>>>>>>>>  
>>>>>>>>>>>                                                     
>>>>>>>>>>>> Caimaniacs,
>>>>>>>>>>>>
>>>>>>>>>>>> Please review the following simple change:
>>>>>>>>>>>>
>>>>>>>>>>>> 4054 beCopy() needs to take parameter for BE description
>>>>>>>>>>>>
>>>>>>>>>>>> WR: http://cr.opensolaris.org/~tsk/4054/
>>>>>>>>>>>> B:  http://defect.opensolaris.org/bz/show_bug.cgi?id=4054
>>>>>>>>>>>>
>>>>>>>>>>>> This has been tested on bld99 with pkg(1) and beadm(1M).
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>> Tim
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> caiman-discuss mailing list
>>>>>>>>>>>> caiman-discuss at opensolaris.org
>>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>>>>>>>>                                               
>>>>>>>>>>>>                       
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> caiman-discuss mailing list
>>>>>>>>>>> caiman-discuss at opensolaris.org
>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>>>>>>>                                                           
>>>>>>>> _______________________________________________
>>>>>>>> caiman-discuss mailing list
>>>>>>>> caiman-discuss at opensolaris.org
>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>>>>                                 
>>>>>> _______________________________________________
>>>>>> caiman-discuss mailing list
>>>>>> caiman-discuss at opensolaris.org
>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>>                   
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>         
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>   

Reply via email to