Mark Martin wrote:
> Ru-Song Vincent Zheng wrote:
>   
>> Hi Mark,
>>
>> Thanks for your review. Please see my inline comments.
>>
>> ----- ??? -----
>> ?: Mark Martin <storycrafter at gmail.com>
>> ??: ???, 2009 ? 6? 18?, ??11:57
>> ??: Re: [sfwnv-discuss] Code review request for jedit after PSARC
>> ???: "rusong.zheng" <Rusong.Zheng at Sun.COM>
>> ??: sfwnv-discuss at opensolaris.org
>>
>>   
>>     
>>> rusong.zheng wrote:
>>>     
>>>       
>>>> Hi,
>>>>
>>>> Per ARC's request, changes to webrev are :
>>>>
>>>> /usr/share/jedit/4.3pre16/      ==>/usr/share/jedit/
>>>>
>>>> sunman/jedit.1                       ==>jedit.1          sunman is 
>>>>       
>>>>         
>>> removed>build.xml.patch                      
>>> ==>patches/build.xml.patch>build.properties.patch           
>>> ==>patches/build.properties.patch>jedit.patch                       
>>>       ==>patches/jedit.patch
>>>     
>>>       
>>>> directory classes is removed for that contents of it are 
>>>>       
>>>>         
>>> duplicated (class files and API docs,  class files already be 
>>> contained in jedit.jar, APIs doc already in doc/API/)
>>>     
>>>       
>>>> jedit is removed from /usr/share/jedit/jedit and moved to 
>>>>       
>>>>         
>>> /usr/bin/jedit>
>>>     
>>>       
>>>> Makefile.sfw, install_sfw and prototype_com are changed accordingly.
>>>>
>>>> Besides that, METADATA file has been changed to meet the new 
>>>>       
>>>>         
>>> requirement. All the others are the same as the last round of review.
>>>     
>>>       
>>>> Please help review them.
>>>>
>>>> Webrev is at:
>>>> http://cr.opensolaris.org/~rszheng/jedit/
>>>>
>>>> PSARC case url is:
>>>> http://arc.opensolaris.org/caselog/PSARC/2009/357/
>>>>       
>>>>         
>>> I'm confused about the script "jedit", and I'll admit it's because 
>>> this is the first java->sfw project I've code reviewed. That, and 
>>> my Make is probably a little rusty.
>>>
>>> It occurs to me that the patch creates a file jedit.sh.  If that is 
>>> true:a) Where is "jedit" that's been referenced as /usr/bin/jedit
>>> b) Is that the appropriate name?
>>>
>>>
>>>     
>>>       
>> Yes, jedit here is a script to run "java -jar jedit.jar". 
>> for a), the script did not exist firstly. I wrote a script and make a diff 
>> to generate the patch, and original script I wrote is removed. When build is 
>> finished, the script will be generated and installed to /usr/bin/. So 
>> /usr/bin/jedit is itself a script and don't refer to other file.
>>
>> for b), I think it is easy to use: user can just type "jedit"  commandline 
>> to run jedit.jar. 
>>   
>>     
>
> You're right, of course. I misinterpreted what I read from the patch.
> Too rusty. Looks fine.
>   
Hi Mark,

Thanks for your review. :)

Rusong

Reply via email to