Re: RFC before sending patches for a few FIXMEs



On Fri, 2004-02-27 at 03:46, Clement Moyroud wrote:

> 1/ Where should Dashbord configuration go ? GConf might be the good
> solution, being the Gnome standard. I have used this layout :

For the most part, dashboard should have as little configuration as
possible.  Any necessary parameters (like the path to the user's browser
directory) should be read from the relevant places in the system.

Some configuration will be necessary, and I think using GConf makes
perfect sense.  For all GConf keys we add, though, we need to add
schemas with descriptions of what they configure.

The big dashboard configuration problem is backend selection.  I think
some of this will be automatic -- figuring out which browser you use by
default, for example -- and some will be user-configurable.  Relevance
filtering may end up changing the semantics of this.

>         apps/dashboard/backends/<backend-name>

Yup, sounds good.

> 2/ the MozillaBookmarksBackend author would like to use XPath for the
> parsing of the html file. To my (limited !) knowledge, that is not
> possible. The bookmarks.html file is not well-formed XML (not being
> XHTML compliant). One could write a SGML parser, but I'm not sure it'll
> be good performance-wise compared to the regexp used actually.

I don't know whether it's possible or not, since I don't know the format
of the Mozilla bookmarks.  Since we have a working regexp now, I don't
see a reason to change it.

> - bugzilla-backend-patch.diff : adds a gconf key for the choice of the
> bugzilla server

This is great, and it's in CVS.

> - geosites-backend-patch.diff : adds the 2 letter state code for
> Mapquest URLs

This code:

+                       while(l != null)
+                       {
+                               string [] fields = l.Split ('\t');

Does not obey the coding guidelines.  You should put the open brace on
the same line as the while statement, and put a space between "while"
and the open parenthesis.

I've made this change and committed this patch.

> - manpages-backend-patch.diff : some manpages on my system (Debian) are
> symlinks to .gz files, and do not end in .N.gz . This patch skips such
> manpages for now. A better one is in the works :)

I'm gonna pass on this one for now.  I'd rather have it die screaming
and remind us this is a problem than fail silently.

> - mozilla-backend-patch.diff : adds 2 gconf keys for the choice of the
> mozilla flavour and the profile name

I think we need to autodetect the user's browser and default profile.

Thanks for your patches!  Nice stuff!

Best,
Nat




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]