1. usr/src/cmd/sg3_utils/Makefile.sfw
- I suggest renaming install-sg3 to install-sfw to keep with the
general convention
- test without the find $(VER) ... statements as they probably aren't
needed and just add
overhead to the build
- Why would you want to find and remove core files at the end of the
'all' target
- don't hard code the path to the shell used to run the script. Use
$(SH) for /usr/bin/sh or
$(SHELL) for /usr/bin/ksh93
- remove the 'test' target if you aren't going to use it
- Do you need to specify the dependency on $(VER)/src with the
'install' target?
- Get $(VER) from METADATA file as suggested by Muktha
2. usr/src/cmd/sg3_utils/install-sg3
- rename to install-sfw
- Use ksh93 as described here::
Roland Mainz wrote:
> use /usr/bin/ksh93 or /usr/bin/bash for install-sfw*
> and add a
> $ set -o errexit # at the beginning and replace
> ". > ${SRC}/tools/install.subr" with
> "source ${SRC}/tools/install.subr" (the idea is to
> catch failures in the script and abort it at that
> point, right now the script will just continue)
- '# /* Portions Copyright 2007 ShivaKumar GN */' <------ Should this
be there?
3. usr/src/pkgdefs/SUNWsg3utilsr/Makefile
- with 'DATAFILES = depend' your asking the build environment to supply
the depend file
which means you don't need to supply your own
4. usr/src/pkgdefs/SUNWsg3utilsr/depend
- This looks like the default depend file so you can remove it as long
as the Makefile specifies
'DATAFILES = depend' (which it does)
5. usr/src/pkgdefs/SUNWsg3utilsr/prototype_com
- I've seen that other packages are already doing this, but why would
you include
/etc/security/prof_attr and exec_attr as part of the package? Maybe
it's something I've missed
but even if it is allowed the way it's being done will just clobber
the existing files along
with any changes made by the user. With Lighttpd we added our entries
to the common files
as you have done, and these updates appear in the build in which our
packages appear. No
need to include them with the package.
I'm new to doing in depth code reviews, but Paul's right we in Sun need
to do more of them so as to move the process along.
Amanda
xiao li - Sun Microsystems - Beijing China wrote:
> Hi Experts,
> I'm responsible for integrating sg3 utilities into opensolaris.
> My code change is available at:
> http://cr.opensolaris.org/~xl222276/sg3utils
>
> You comments will be highly appreciated.
>
> Thanks and regards,
> -Xiao
>
>
>
> _______________________________________________
> sfwnv-discuss mailing list
> sfwnv-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>
