1 |
Thanks for looking at this Christian. |
2 |
|
3 |
On 12/29/2009 03:54 AM, Christian Faulhammer wrote: |
4 |
> Looks fine so far. What puzzled me is the documentation of the SLOT |
5 |
> variable. What is the motivation to do so? |
6 |
|
7 |
It would be possible for two different versions of a Control to be installed |
8 |
and work with gdesklets-core. An id string is calculated based on the |
9 |
interface to the Control, and this string is used by the Desklet to reference |
10 |
the Control. So if two versions of a Control have different interfaces, |
11 |
Desklet "A" could use the older version and Desklet "B" could use the newer |
12 |
version. The core program would allow it and I'd like to provide the support |
13 |
for doing this in portage. This is why obz and I originally chose to install |
14 |
to a directory identified by the Control name and its id string (i.e. |
15 |
"/usr/lib/gdesklets/Controls/ImageSlideShow_cdu4rwmfeuauyw2wwuaczc3lr-2"). |
16 |
|
17 |
> * Sometimes you give a default on undefined ROOT variable, sometimes |
18 |
> not. Please make it consistent for cosmetic reasons. |
19 |
|
20 |
Okay. I guess since the PMS specifies that it "must be non-empty and end in a |
21 |
trailing slash", I will remove the default on undefined. |
22 |
|
23 |
> * addwrite "${ROOT}/root/.gnome2": Is this unconditionally necessary? |
24 |
> Or could a "boolean" in the ebuild be set to activate it? |
25 |
|
26 |
Unfortunately, it seems to be unconditionally necessary. Bug #128289 [1] |
27 |
still seems to apply upstream (see comment #18). I have not looked into glib |
28 |
source code since this issue was originally "fixed" [2]. |
29 |
|
30 |
> * DISPLAY variable export could be done with the assignment. Or is the |
31 |
> export always needed? |
32 |
|
33 |
Done. I would like this to go away, but haven't touched the |
34 |
gdesklets-control-getid script in a long time. The easy way was to export |
35 |
DISPLAY. |
36 |
|
37 |
> * Is the file name LICENSE always used for the license or is COPYING |
38 |
> for example also possible? |
39 |
|
40 |
I believe it will always be LICENSE if packaged according to upstream [3]. I |
41 |
believe it is not allowed to be something else in order to be uploaded to the |
42 |
main site, but as the link is down atm I can't check it. |
43 |
|
44 |
> * einfo "Installing Control ${CTRL_DIRNAME}": Is not mirrored in the |
45 |
> desklet branch of the if clause. |
46 |
|
47 |
True. Originally it was mirrored, but some Desklets in the tree (I'm thinking |
48 |
of x11-plugins/desklet-ftb right now) contain multiple Desklets (I can't |
49 |
remember if this is allowed any more with the requirements specified in [3], |
50 |
but I'm open to suggestions). I thought it was a good compromise to have the |
51 |
einfo lines in pkg_postinst. The difference between the two is that multiple |
52 |
Controls cannot be packaged together (both under the new upstream packaging |
53 |
requirements and for this eclass to work). |
54 |
|
55 |
Patch to the original revision is attached. |
56 |
|
57 |
-- |
58 |
Joe |
59 |
|
60 |
[1] http://bugs.gentoo.org/show_bug.cgi?id=128289#c18 |
61 |
[2] http://bugs.gentoo.org/show_bug.cgi?id=126890 |
62 |
[3] http://gdesklets.de/index.php?q=node/2 |