Petr, See below for my quick skip through comments ...
Paul Petr Sumbera wrote: > > I have prepared webrev for Wireshark integration to Solaris Nevada > (PSARC PSARC/2007/334). Can someone do code review for me? > > http://cr.opensolaris.org/~xsumbe00/6567201-wireshark/ === Start of Comments ==== 1. CDDL HEADER and top of files Cosmetic: you might want to update the tops of the files so that they conform to that in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; Most have just got a missing line-space, but there are a few that need more work. 2. usr/src/cmd/wireshark/METADATA Make the NAME: field more descriptive 3. usr/src/cmd/wireshark/Makefile.sfw Do you need to run 'protofix' on the stuff installed by 'make install' to correct permissions/ownership? (you probably will go after fixing (7)) 4. usr/src/cmd/wireshark/sunman-stability Is "Interface Stability Unstable" correct? 5. usr/src/pkgdefs/SUNWwireshark/copyright & usr/src/pkgdefs/SUNWwiresharkr/copyright Do you need to add any owner copyright lines near the top of the file, eg. the copyrights from src files in the unpacked src tarball? 6. usr/src/pkgdefs/SUNWwireshark/depend Move Copyright lines to after the 'CDDL HEADER END' header. 7. usr/src/pkgdefs/SUNWwireshark/prototype_com Remove the write permissions from the files installed into /usr 8. usr/src/pkgdefs/SUNWwireshark/prototype_i386 & usr/src/pkgdefs/SUNWwireshark/prototype_sparc & usr/src/pkgdefs/SUNWwiresharkr/prototype_i386 & usr/src/pkgdefs/SUNWwiresharkr/prototype_sparc Cosmetic: add the SUNW pkg name as comment, just for consistency with other pkg files, example see ... "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/prototype_i386"; 9. usr/src/pkgdefs/SUNWwiresharkr/pkginfo.tmpl Add pkg version to the end of the DESC= line, eg. DESC=""....... (n.n.n)" === End of comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
