Hi Karen, Thanks for taking the time to review. I know it's a big document, so we really appreciate you taking the time to review it. Replies inline.
Karen Tung wrote: > Hi Sue and team, > > Great document with a lot of details. My comments are below. > > General comments: > > 1) In many sections of the document, I think it will be easier to > read and understand if the details section of the function comes > first, and then, > all the input , output and errors. That way, the input, output > and error makes more sense, based on one's understanding of > what the function is doing. This is a good point. On the other hand, I think it would be easier to go back and reference the document if the inputs/outputs/errors are listed first. So I don't know if I want to switch it or not yet - a 3rd opinion on the matter would be good. > > 2) It would very useful to have a diagram showing all > the different pieces. How things flow, which of the different > libraries will be used when...etc.. I agree. We will try to get some visual aids prepped, including some UML diagrams that will describe the data structures a bit better. > > 3) The document focuses on the new libraries and functions that will be > introduced. It will be useful to list other install libraries that the > text installer implementation will use, but will not need to modify at > all, such as libti, and libtransfer, and libict. These are called out by the functions that require them. For example, 2.5.8 prep_transfer indicates that it uses libtransfer's "do_transfer" function, and 2.5.9 run_ICTs lists the ICTs that will be needed. Looking at it again, I think we need to be more explicit in the case of how we will call do_transfer. The above-mentioned diagrams should also show how and what other functions are called. > > 4) There's no mention anywhere in this document that this will be > a cpio-based install of everything on the media, > not a pkg-based install. Additionally, user is not allow > to select which packages to install or not. We can call out more specifically that it will be CPIO based (and that this means that the software on the media is what will be installed, and customizations must be done using pkg after reboot). > > 5) Which package will the text installer be delivered in? A new package? > An existing package? Where will the text installer program reside, and > what's it's name? SUNWtext-install /usr/bin/text-install This information will be added. > > > 6) Changes are made to the LiveCD and GUI installer so it is accessible. > Will this design allow the text installer to be accessible as well? > Do you know any changes is needed to make it accessible? We're awaiting confirmation on whether or not additional work is needed to make it accessible. Preliminary feedback indicates that terminal based programs are accessible by nature. > > Specific Comments: > ----------------------------------- > > - In the very beginning, before the Table of Contents > The URL for the Text Installer project is not correct. Good catch. Not sure how that got clipped. > > - 2.1.2, last paragraph, second sentence, "subsequent screens will not > need to "recreate" from scratch....." > > Does this mean each screen is not individually pre-drawn, but it is drawn > at real time, and all needed data in the fields are re-populated at > real time? In this context, only the base "layers" provided by MainWindow are being retained - the header, footer and blank central area objects are owned by MainWindow, which means that individual screens do not need to build new headers, etc, when they appear. > Did we do any measurement on the amount of memory it will save to use > this approach verses the approach of having each of the screens > pre-drawn, > and kept in memory? In many cases, the screens can't be "pre-drawn" - e.g., you can't make the partition screen until you know what the disk is, you can't create the list of time zones until the user selects a region/country, etc. > Also, did we make sure that the user experience of > doing this real-time screen drawing will be OK for slower machines with > smaller amount of memory(512MB), when the user goes back and forth > between screens a lot? This is less a memory issue, and more a CPU speed issue. As a data point, thought, the preliminary binary I sent to QE and the DDU team reports about 11 MB in use - within a few KB of the most basic of Python scripts, due to the overhead of running in Python. > > 2.1.6: In the functional spec(section 3.2), it says that when the user > exit the installer, it will display a small menu of choices. In this > design > specification, there's no discussion at all about how that small menu > is displayed? What program is called to display that menu...etc.. > Please elaborate on that. See section 2.8. > > 2.1.7: When reporting errors, will the error be reported immediately? > ie: If I type an invalid value > in input field1, will I be immediately warn about the error as I move > my focus to another > input field? Or will all the error checking be done when I try to > click next? > Also, will I be allow to go to the next screen if I got invalid errors? > Please clarify in this section. Please see the UI spec. I will point to the UI spec again here. > > 2.1.7: Can you give an example on how one will get to this situation > of making an invalid selection? If we are giving people choices, is it > possible to always only give them valid choices? Most invalid selections are impossible situations (for example, the UI spec calls for not allowing the user to even highlight a disk that is not at least as large as the minimum size). The only example of invalid selection I can think of would be trying to continue past the partition screen without creating a Solaris partition. > > 2.3.2: What about default keyboard and default language? That should > also be included > in the installationProfile object. Good point. These were initially left out because those details were selected during the boot process, but they should be part of the InstallationProfile as well (just not editable by the user). > > 2.3.2.4: why is hostname part of UserInfo? That is not related to a > user. Moving this to NetworkInfo. While I'm here, it makes more sense to change UserInfo to just be name and password. InstallationProfile will have an array of UserInfo (in the case of the Text Installer, only 2 UserInfo's will be in the array - one for "root" and one for a single additional user). > > 2.4 and 2.5: There is the Required Library Functionality and > Non-Library functionality > section. Why group them as such? Looks like some of the functions in > both sections > could be used by other installer as well. Will functions from these 2 > sections be > delivered as separate libraries or the same library? Functions in 2.4 Required Library Functionality should and will be made available to other installers. Functions in 2.5 Non-Library Functionality will be implemented and private to the Text Installer. In the case of get_[min|recommended]_install_size, this data is read from the .image_info file and is very small code, so it was decided that it would be implemented internally via Python rather than going through the overhead of a library call (that would likely dive through a bridge to C code). 2.5.4 configure_network is somewhat related to the big discussion regarding DDU, setting things in a common way, and so forth. This could be worth putting into a common location for use by AI. 2.5.5 through 2.5.9 are the Text Installer "module" in terms of CUE - they define what a "Text Install" is in the framework of what library calls are made, what data they use, etc, and as such, are project private. > > 2.5.5.4: Does "prep_transfer" include the actual transfer of bits? > Based on the description > in 2.5.8, I think so, but I think the choice of name is not so good. Yes, it's a bad name. I was trying to avoid a naming clash with libti's do_transfer call, but prep_transfer is a misleading name. Would "perform_transfer" be preferable? > > 2.8.3: strictly speaking, 3 new files will be added to the > SUNWdistro-const package. > text_installer_x86.xml, text_installer_sparc.xml and > text_generic_live.xml Ok, that will be called out explicitly. > > 2.8.3: Since the grub menu will only contain 1 entry, the exiting > grub_setup.py finalizer > script will need to be modified. At this time, this script will > insert at least 3 entries > into the grub menu. The default value will be set, and timeout set to 0. In this way, it is irrelevant how many GRUB entries are on the disk. However, I'd be happy to file a separate RFE to enhance grub_setup.py to make adding those entries optional - but strictly speaking, it is unimportant. - Keith > > Thanks, > > --Karen > > Sue Sohn wrote: >> We have posted the design document for the Text Installer at: >> >> http://www.opensolaris.org/os/project/caiman/TextInstallerProject/design_doc_v1.4.pdf >> >> >> >> Please review and provide comments by COB Tuesday, September 15th. >> >> Thanks, >> Sue >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
