Thanks Brock for the review, much appreciated, comments below and 
updated webrev at:

http://cr.opensolaris.org/~jmr/pm_startpage_only_Feb05/

JR

Brock Pytlik wrote:
> General comments:
> What, if any, assumptions are being builtin because, right now, 
> dynamic update of this start page isn't a requirement? Ie, if 
> marketing came back tomorrow and said "we need dynamic update" which 
> bits of existing code would have to be rewhacked? Personally, I see 
> little point in a static splash page while a dynamic one could have 
> some real use I would think.
There are no assumptions at the minute. When Webkit is available what 
will happen is the GtkHTML component will be changed in the Gnome 
distribution to use it, so from our standpoint scripting will just start 
to work in these pages. We will still be using the same handlers to deal 
with mouse over and link click events. So at this stage by updating the 
SUNWipkg-gui-data package marketing could refresh these pages with a lot 
fancier HTML.

If there was a requirement to more frequently update the startpage.html 
then this could be done as part of the current cron job we run to do the 
pkg refresh. We'd need to modify the code to look in 
/var/pkggui/startpage,/<locale>/ as this is where the updated files 
would need to be placed, before looking under the install directory. A 
simple change if required.
The feedback we have so far is that they do not want to have yet more 
properties to maintain such as a custom dynamic StartPage and so would 
prefer to point to external pages which are resourced and maintained at 
present, with periodic updates to reflect branding changes.
>
> packagemanager.py:
> It would be nice if START_PAGE_BASE simply filled in the slot of 
> START_PAGE_LANG_BASE with an appropriate value ("default"?) rather 
> than duplicating the path, that way if the files moved from usr/share 
> to something else, there's a single point of change.
>
> Same comment for START_PAGE_HOME, that should probably be used to 
> substitute into the two other variables.
>
Changed as you suggested, now will have C as default locale and just use:

START_PAGE_LANG_BASE % (self.lang, START_HOME_PAGE)
> line 169:
> Shouldn't self.in_startpage_startup be equal to show_startpage?
>
Thanks nice catch - changed
> 182:
> remove spaces
>
Done
>
> 381-397:
> Is there a reason that the "C" language isn't used as the default 
> start page? That's what lines 366-373 seem to be establishing anyway. 
> Given those lines, can't we declare the C locale to be the default and 
> just dump lines 384-386?
>
We can have a valid locale but there may not be a startpage available 
for it, so you still need to try with the given locale and if that fails 
try with the "C" locale which is now our default as per your suggestion 
above.
> 437:
> I think this should be moved above the try block.
Done
>
> 463-470:
> Given that only one substitution happens outside of html code, using 
> straight % substitution here is probably ok, though I would like 
> someone with more html/internationalization experience to verify that 
> for me.
As long as we are using the _() builtin for l10n support we should be ok 
I think, Fujiwara will let us know if we are not.
>
> 476-477 and 504-509:
> Why ignore links without pm-action instead of treating them (for 
> example) as external links?
Nice idea, changed so if no action specified defaults to external browse.
>
> 484-498 and 515-538 sure look similar, maybe there's some commonality 
> to be extracted from them?
Folded them into one function, makes it little harder to read, which is 
why I had them separated in the first instance, but will be easier to 
maintain if new actions are added, as we only have one handler for them 
to update.
>
> Thanks,
> Brock
>
> jmr wrote:
>> Hi following is a webrev for changes to PM,
>>
>> http://cr.opensolaris.org/~jmr/pm_startpage_only_gate_Feb04/
>>
>> Enhancement: 6355: Add StartPage to PM
>>
>> Also folded in one line change for Bug: 6269: PackageManager does not 
>> quit after cancel update
>>
>> JR
>>
>>
>> Change Summary 6355:
>> Start Page
>> -----------------
>> In line with other desktop apps like NetBeans and Thunderbird a 
>> StartPage is
>> being added to the PM.
>>
>> On startup the StartPage is displayed where the package list is 
>> displayed.
>> - If a user starts searching, or switches categories the list view 
>> replaces the
>> StartPage.
>> - The user can always get back to the StartPage from the Help->Start 
>> Page menu
>> item.
>> - A Preferences Dialog will be added to allow a user to turn off 
>> display of the
>> StartPage on startup if they wish (refer to bug 6354 
>> <http://defect.opensolaris.org/bz/show_bug.cgi?id=6354>). They can do 
>> so now by
>> modifying a gconf key setting.
>>
>> The StartPage is a placeholder that marketing can modify as they see 
>> fit before
>> a release. At present there is no plans to dynamically modify the page.
>>
>> The StartPage is hosted in a simple HTML renderer control that just 
>> allows HTML
>> to be displayed, along with images and CSS support. There is no 
>> scripting
>> support.
>> At present links can be added to the page to allow browsing within the
>> StartPage pane or externally by launching the default browser. At 
>> present the
>> internal browse is only used to handle error conditions and the main 
>> StartPage
>> has images which are all linked to external OpenSolaris web pages.
>>
>> It is envisaged that this static Start Page will bring users to specific
>> OpenSolaris properites and by having ips MimeType support on the 
>> server (refer
>> to bug 6352 <http://defect.opensolaris.org/bz/show_bug.cgi?id=6352>) 
>> the user will be able to add additional repos and/or packages by
>> just clicking on links on these external web pages.
>>
>>
>> _______________________________________________
>> pkg-discuss mailing list
>> [email protected]
>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>   
>

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to