[yocto] PULL] ADT installer script implementation, Liping Ke

[Zhang, Jessica]

Richard Purdie wrote:
> Hi Liping, Jessica and team,
> 
> On Thu, 2010-12-09 at 17:28 +0800, Ke, Liping wrote:
>> Pull URL: git://git.pokylinux.org/poky-contrib.git
>> Branch: lke/adt_installer_initial
>> Browse:
>>
http://git.pokylinux.org/cgit/cgit.cgi/poky-contrib/commit/?h=lke/adt_instal
ler_initial&id=a06769b01d7b18894ad413daa8159f74c90f6c0f
> 
> 
> I had a quick look over this and have a few comments. I'm in critical
> review mode and don't have a lot of time so please don't take any of
> this feedback the wrong way, I just want to give you honest first hand
> first time user perception.

Really appreciated your time and feedbacks.

> 
> The first thing I read was in the patch was:
> 
> +# following are for testing in my machine. it will be moved when
> +# going into code repository
> +export http_proxy=""
> +export HTTP_PROXY=""
> +export HTTPS_PROXY=""
> +export https_proxy=""
> +export ftp_proxy=""
> +export FTP_PROXY=""
> 
> which it evidently wasn't! ;-)
> 
> The rest of that file suggests it probably shouldn't be there
> actually? 
> 

Sorry for this obvious mistake. Need to do a better job before sending our a
review request next time

> I then decided to try it.
> 
> cd /
> root at rex:/# /rphome/poky/scripts/adt-installer/yocto_installer
> /rphome/poky/scripts/adt-installer/yocto_installer: line 207:
> .config: No such file or directory [snip lots of similar errors]
> 
> So I'd suggest it looks at the location of the main script and then
> uses 
> this to find its config files.
> 
> It then found the presetup .config file and took those values as the
> defaults. This was not obvious to me, it should perhaps really state
> where its getting that config from.
> 
> I removed the .config file. That didn't help me so I removed
> yocto_installer.conf. This didn't help me much either.
> 

That definitely a bug regarding how to locate .config that we need to
address.  As to the different config files, the .config is on purpose non
user editable that's why we set it as hidden file by default.  And the
yocto_installer.conf is the customizable config user should change and we
have prompt for the user as regard to the configuration values to be used
and where they're from along the installation process...

> Now, reading the script I suddenly realise the user doesn't get
> prompted 
> to be able to change the values, you can only set these in the config
> files.
> 
> I tried hitting Y for fun despite knowing it would crash and burn.
> Those 
> error messages were good so kudos there :)
> 
> I think it would make most sense for the user to be prompted for unset
> values with "enter" selecting a sane set of defaults.
> 
Agree and will make improvement on that..

> So in summary, I think the right pieces are there in terms of
> configuration but the user interaction needs a lot more thought. I
> didn't try an actual installation as I don't have a local setup to
> allow 
> it.
> 
> For the next review, I'd like an opkg repo to be available somewhere

Lianhao has a testing opkg repo setup on his machine which mimic the release
package repo and we've been using it test things out

> to 
> test against and for you to provide an installer tarball as an end
> user 
> would receive for people to evaluate.

Yes, and will attach the tar ball with in the next review request
> 
> Cheers,
> 
> Richard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 8455 bytes
Desc: not available
URL: <http://lists.yoctoproject.org/pipermail/yocto/attachments/20101209/36ff6e26/attachment.bin>