Jeff Trawick wrote: > Amanda Waite wrote: >> Jeff Trawick wrote: >>> sunanda menon wrote: >>>> Hi , >>>> Please do a Code Review request for the upgrade of MySQL from >>>> 5.0.67 to 5.0.77 (CR 6808952) >>>> at http://cr.opensolaris.org/~sunandam/6808952/ >>>> >>> it looks okay to me, though it would be good for somebody more >>> familiar with packaging to have a look as well ;) >> >> I'd do it, but I'm used to doing reviews on sfwnv-discuss and you >> guys might think some of the suggestions that I make are excessively >> nit-picky. > What types of review comments are there (staying away from the term > "nit-picky")? > > * identification of changed code that introduces a problem > (functional, legal, whatever) > ** must resolve in the current review package before integrating > * identification of changed code that could be improved > (maintainability and/or consistency) > ** track via separate CR if integration is high priority and there's > no time to resolve immediately > * identification of existing problem Comments that support the following: - Consistency of code both in terms of layout and content across all components being integrated/updated in SFW. - Making use of variables wherever possible to minimise the number of changes required during updates, this generally means pulling data from the METADATA file - Taking all recommended steps that will ensure that problems that do occur during the build process do result in a failure - Eliminating unnecessary steps during the build process - Making sure that files and directories have the correct permissions, that files are installed to the proto area during the build in the correct way - Verifying that packages are correct, that they use the correct classes for various files, that the fields in pkginfo are correct and that dependencies are correctly specified - Making sure that CDDL headers are correctly specified - Making sure that no changes would adversely affect the build. i.e.: removing stuff that's already there
Because of the work of the reviewers on sfwnv-discuss, the standard of initial code reviews is now very high as is the standard of the final approved code. Also many potential issues both major and minor have been caught in advance of the RTI submission, sometimes this just prevents RTIs from being rejected, sometimes it prevents broken integrations or broken builds. What's always bothered me though is how to apply this to updates. If it worked before then why make changes that may add additional QA overhead over and above that required by updating the code? I take the view that there are some things that should be left alone, although I'd still point them out. Amanda > > _______________________________________________ > > > webstack-discuss mailing list > webstack-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/webstack-discuss
