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

Bug#821270: Review of firefox-branding-iceweasel



Hi,
(just answering the alioth part)
https://qa.debian.org/developer.php?login=pkg-mozext-maintainers@lists.alioth.debian.org
this should be the maintainer

and the account has to be created on alioth.d.o, and then join the group
https://alioth.debian.org/projects/pkg-mozext

cheers,

G.




Il Martedì 19 Aprile 2016 16:51, nord-stream <nord-stream@ochaken.jp.eu.org> ha scritto:
On 18/04/16 20:20, Sean Whitton wrote:
> The package is mostly fine.  Here are some points:
>
> - binary package name should be xul-ext-iceweasel-branding or similar

I thought of creating packages named firefox-branding-iceweasel,
thunderbird-branding-icedove, etc. I am aware of the naming convention,
but these extensions are not much like extensions but just branding
packages. (In fact it Provides: xul-ext-iceweasel-branding.)
Is that naming mandatory? I also found many of xul-ext-* packages do not
include a single XUL file. (Neither does firefox-branding-iceweasel.)
More discussion at:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

> - is it possible to generalise this to restore both icedove and
>   iceweasel branding in one binary package?  (icedove will soon become
>   thunderbird)

It may be possible, but many people use one and not another, so one
binary package may not be desirable. One source package may be a good
idea, though.

> - don't install the MPL-* files using debian/docs.  Instead, include 
>   the full license text in debian/copyright.

firefox-esr package seems to do this. Do you mean that it is not
appropriate for a branding package?

> - as Gianfranco suggested, this should be team maintained.  Your name
>   should be in the Uploaders: field in debian/control, and Maintainer:
>   should be the Mozilla extensions packaging team.  You should upload
>   the git repository to the Mozilla extensions team section of alioth.

That seems right. Please tell me more.

>   Do you have an alioth account?

I don't. What does it mean? I'm willing to do what's needed.

> - the long description is not, IMO, appropriate.  You should include
>   the history of the package in the README.markdown, and just give a
>   terse description of what it does in the long description (or at
>   least in the first paragraph of the long description)

I'll consider this point later.

> - on my machine, the package doesn't change the application icon --
>   see the top of the attached screenshot.  Maybe you can't fix that,
>   though.

Not possible with an extension. Doable with a .desktop file, I think.

On 19/04/16 02:18, Sean Whitton wrote:
> I just took a closer look at your debian/rules file.  You don't need the
> boilerplate.  This is enough:

Done. Will upload later.


> However, since this is a native package, would you consider editing your
> Makefile so that dh_xul-ext can do your whole build for you?  Take a
> look at the source package y-u-no-validate.  It uses this main Makefile
> target:
> 
> ,----
> | %:
> |     dh $@ --with xul-ext --buildsystem=xul_ext --sourcedirectory=src
> `----
> 
> There is an override, but it's just something minor.  This works because
> the source code is organised in a standard way, but your code seems to
> be organised in a non-standard way which is why you need a complex
> Makefile and dh overrides.

The complex part of Makefile is from Iceweasel package. Although most
extensions' Makefiles just pack files into .xpi, it generates files from
source files. This tiny override just saved me of hours of studying more
about customizing dh_xul-ext.

Thank you for reviewing,

--
nord-stream


Reply to: