Faramarz,

Faramarz Jalalian wrote:
> 
> Thanks for your careful code review. Really appreciate it. 

That's okay :-)

>  My comments  below:

I've added some replies ...

> Paul Cunningham wrote:
>> Faramarz Jalalian wrote:
>>>
>>> Would you please review the changes to include CMake into 
>>> OpenSolaris? You can find the webrev at:
>>>
>>> http://cr.opensolaris.org/~jalalian/cmake/

.. cut ...

>>
>> 1. usr/src/cmd/cmake/Makefile.sfw
>>    Why have you put this in ? ...
>>     "# Portions Copyright 2008 Faramarz Jalalian"
> I just followed the what was done for cmd/expect/Makefile.sfw. Also the 
> comment header instructs that
> 
> "If applicable, add the following below this CDDL HEADER, with the
> #  fields enclosed by brackets "[]" replaced with your own identifying
> #  information: Portions Copyright [yyyy] [name of copyright owner]"
> 
> And I wasn't sure if it's applicable or not, so I added the line to be 
> safe. I think it doesn't mean anything other than
> I have created this file.


I don't think it is needed so I would remove it - as it's just clutter

.. cut ...

>> 5. usr/src/pkgdefs/SUNWcmake/depend
>>    This looks like the default 'depend file! If you have
>>    no other dependencies then you should remove this file
>>    and add "DATAFILES= depend" to your SUNWcmake/Makefile.
>>
>>    Have you checked you have no other pkg dependencies? If
>>    you have keep this file and add them here. Also ..
>>
>>    move Copyright lines to after the "CDDL HEADER END"
>>    header.
>>
> I prefer to keep this in case that some kind of dependency is discovered 
> later.
> I moved the copyright lines. SUNWexpect and SUNWdiffstat also have the 
> same problem.

They shouldn't have the 'depend' file either if they don't have any of 
their own dependencies - a BUG in my opinion.

If you have checked that you have no extra dependencies then you should 
remove yours as well (as above)! So in my opinion, if you keep it, you 
would be putting-back another BUG.

Paul

... cut ...

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products

Reply via email to