Thanks for the review dermot.
On 09/02/11 11:09, Dermot McCluskey wrote:
Hi Matt,
Mostly nits:
auto_install.py:
line 74 - you haven't made any changes that require
importing varshared
Registering of the VarSharedDataset checkpoint requires the varshared
module to be imported.
line 99 - how come the list CHECKPOINTS_*BEFORE*_TI
includes TI itself?
A good point I suppose, naming of the variable is possible poor, and
should be CHECKPOINTS_UPTO_TI or something similar. but it's been called
this and including TI checkpoint since CUD/AI was integrated last May.
If you feel strongly I can change it.
line 310: if GUI and Text, you now pause before VarShared.
Should that not also be the case in AI?
The options here are very specific to stop before or after Target
Instantiation. Depending on whether -i or -I command line argument has
been passed in. This is unique to AI.
gui-install/src/__init__.py
line 47, nit: could you take the opportunity to alphabetically sort
the imports within editing this statement?
Done
line 50 - why move the definition of TARGET_INIT from
gui_install_common to this module?
As it's not referenced anywhere else, and that's what all other non
referenced checkpoint definitions do.
progress_screen.py
line 49, nit: could you take the opportunity to alphabetically sort
the imports within editing this statement?
Done
ti_install.py
line 244 - nit: can you remove unneeded trailing \ while you are
editing this statement?
Done
varshared.py
line 231 - remove commented code
Actually I'd prefer to leave this here, as this will effectively be
uncommented when /var/shared is implemented, I've added another comment
above the for loop to indicate this.
New webrev posted :
https://cr.opensolaris.org/action/browse/caiman/mattman/7048015.7049157.7049160.1/
cheers
Matt
- Dermot
On 09/01/11 18:22, Matt Keenan wrote:
Hi,
Can I get some eyes to look at the latest revision of the /var dataset
bugs :
7048015 AI should create a separate /var and shared area
7049157 Text installer should create a separate /var and shared area
7049160 GUI installer should create a separate /var and shared area
webrev:
https://cr.opensolaris.org/action/browse/caiman/mattman/7048015.7049157.7049160/
Originally coded to create both a in_be /var dataset and a globally
shared /var/shared dataset, this has been changed to simply create
/var in_be.
This code has been through code review before, but as we are so close
to final build, another scan might be no harm.
All the code required for creating /var/shared is still in place just
not called.
/var/shared work will be undertaken for s11u1 timeframe.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
