Hi Jack, * Jack Schwartz ([email protected]) wrote: > Hi Glenn. > > Here are my comments on the second code review. > > On 07/12/10 05:05 PM, Glenn Lagasse wrote: > >[re-sending as obviously it would help if there was a webrev for people > >to review] > > > >The ON SNAP Integration project is ready for a code review. Since we're > >moving existing code in slim_source into ON I've tried to make things a > >little easier to review. > > > >I have two webrevs/workspaces for review. The first workspace is based > >on slim_source and is sync'ed with Jack's latest changes that he's > >already got a review out for. This workspace has the file differences > >for the existing files that are moving from slim_source into ON. This > >workspace should be used to review only files that aren't "new" > >as part of the integration into ON*. That list of files is: > > > > usr/src/cmd/beadm/BootEnvironment.py > > usr/src/cmd/beadm/__init__.py > > usr/src/cmd/beadm/beadm.py > > usr/src/cmd/beadm/messages.py > > usr/src/lib/libbe/be_activate.c > > usr/src/lib/libbe/be_create.c > > usr/src/lib/libbe/be_list.c > > usr/src/lib/libbe/be_mount.c > > usr/src/lib/libbe/be_rename.c > > usr/src/lib/libbe/be_snapshot.c > > usr/src/lib/libbe/be_utils.c > > usr/src/lib/libbe/be_zones.c > > usr/src/lib/libbe/libbe.h > > usr/src/lib/libbe/libbe_priv.h > > usr/src/lib/libbe_pymod/libbe_py.c > > usr/src/pkg/manifests/install-beadm.mf > > > >The webrev for this is: > > > >http://cr.opensolaris.org/~glagasse/hg_beadm_1_noinstallgrub/ > > > >* Note: This workspace/webrev does *not* contain all of the changes to > >actually remove beadm/libbe from slim_source. That will be done > >seperately. > > > >The second workspace is based on the ON gate and is the 'final' workspace > >which includes all of Jack's changes as well as my changes for the project. > >This workspace should be used to review only files that are "new" as part > >of the integration (although you can also look at it for a feel of what is > >going where once everything is transferred). That list of files is: > > > >added: > > usr/src/cmd/beadm/Makefile > > usr/src/cmd/beadm/__init__.stub.py > > usr/src/lib/libbe/Makefile > > usr/src/lib/libbe/Makefile.com > > usr/src/lib/libbe/common/llib-lbe > > usr/src/lib/libbe/common/mapfile-vers > > usr/src/lib/libbe/i386/Makefile > > usr/src/lib/libbe/sparc/Makefile > > usr/src/lib/pylibbe/Makefile > > usr/src/lib/pylibbe/Makefile.com > > usr/src/lib/pylibbe/common/mapfile-vers > > usr/src/lib/pylibbe/i386/Makefile > > usr/src/lib/pylibbe/sparc/Makefile > > usr/src/pkg/manifests/SUNWbeadm.mf > >modified: > > exception_lists/interface_check > > usr/src/Makefile.lint > > usr/src/Targetdirs > > usr/src/cmd/Makefile > > usr/src/lib/Makefile > > usr/src/lib/libsecdb/exec_attr.txt > > usr/src/lib/libsecdb/prof_attr.txt > > > >The webrev for this is: > > > >http://cr.opensolaris.org/~glagasse/onnv-clone > > > >I'd like to get comments no later than noon on Friday July 16th. If you > >need longer, please let me know. Also please drop me a line as soon as > >possible if you plan to review these changes (if you've already told me > >you will, no need to do this obviously). > Submitted today with your approval... > > Both mapfile-vers files: > > These don't seem to follow the format laid out in the readme > usr/src/lib/README.mapfiles. A quick inspection of some other > mapfile-vers files show that those file follow the README's format.
Fixed. > exec_attr.txt: just curious: why is gid=bin needed? That is copy/pasted directly from the existing rbac entry for beadm in slim_source. I presume that it's there intentionally and so left it. > SUNWbeadm.mf: pkg FMRIs reference b133. Is this OK? (Perhaps there > some automated tool in the gate that updates these with each build?) Again, another file copy/pasted from what exists in slim_source. My (limited) understanding is that 133 is correct and refers to when the 'great renaming' took place. > Otherwise, looks good. Thanks Jack! Glenn _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
