[Bug 181635] Re: Various fixes in debian dir

Loïc Minier lool at dooz.org
Mon Jan 14 10:16:30 GMT 2008


Hi Julien,

0) I had a look at your proposed changes, but since I don't know AWN at
all, I need a little more background: was this change discussed
upstream?  Is the patch from upstream or blessed by upstream?

1) Could you explain the rationale for the move of awn-manager to
Depends?  (Perhaps Breaks would be a better fit?)

2) On the patch itself, it might be a temporary workaround, but I still
want to note a couple of things:

a) you might want to follow the upstream coding style when patching
upstream sources, for example you mix tabs and spaces for indentation,
you use "if (...) {" and "if (...)\n{", you use "/*comment*/" and "/*
comment */" etc.

b) I see fields to access .desktop files are mostly used via #defines
(e.g. GNOME_DESKTOP_ITEM_TYPE), perhaps you want to #define some macro
to "X-AWN-AppletType" as well?

c) usually, one uses stderr to output error messages like the one you
added; you could change the printf to fprintf and use stderr instead?
Perhaps upstream code has some logging facility / macros for such
messages?

d) the error message isn't very helpful when the name field isn't set
("Please inform the developer(s) of the applet 'Unknown'[...]"); I
recommend you include the name of the .desktop file in the message or
use only that instead of the name (the name doesn't really matter to
report it)

Bye,

-- 
Various fixes in debian dir
https://bugs.launchpad.net/bugs/181635
You received this bug notification because you are a member of Ubuntu
Sponsors for universe, which is a direct subscriber.



More information about the Ubuntu-universe-sponsors mailing list