[evolution-patches] Re: default-mailer patch





Looks prety good at a first glance.

I don't think we realy need the 'ask default' option in the mail configuratin window though.  To me it just seems like clutter for an option very few people, if anyone, would ever want to reset.  If they do, there's always gconf-editor, assuming the key is properly documented - you should add a gconf schemas file for this purpose (although to be honest i don't know how you do that off the top of my head).

You should probably also prefix the public symbol names with the org_gnome_xxxx stuff, to avoid name clashes (these will become public symbols in the whole evolution application).  (i..e the callbacks).

Thanks,
Michael

On Thu, 2005-03-17 at 23:36 +0200, Jonathan Dieter wrote:
I took your advice and changed a number of things.  Any other thoughts?

Not Zed wrote:

>
> Hi,
>
> Instead of doing below, you should probably use 
> em_utils_prompt_user().  It presents this type of question in a 
> standard way (i guess i didn't look closley enough to see if that had 
> been set).  I think you can use it - it has a funny api so it might 
> not be usable in this specific case.  If not, then please copy its 
> implementation - i.e. by using identical strings and layout.

Done.

>
> default-client-mail-check is a bit of a mouthful, how about 
> "default-mailer"?
>
Done.

> There are some stylistic things that you should try to follow closer 
> to the rest of the code - but to be honest i couldn't care less for a 
> plugin.  e.g. dialogDefaultEmailApp - why not just call it "dialog".
>
I've changed all variables to at most two words.

> I guess it is just about there though.
>
> It would be better to use the newer method of i18n for error files too 
> - but i wont decline the patch based on that.  It isn't in the 
> documentation properly, it is only in the eplugin skeleton i posted to 
> the evolution blog several months ago.

Do you have an example file somewhere that I could see?  I looked at all 
the .xml files in the errors subfolder and it looked like all of them 
were in the old format.

>
> Thanks,
> Michael
>


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