[Bug 1054941] Review Request: esteidfirefoxplugin - EstEID browser plugin for digital signing

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

 



https://bugzilla.redhat.com/show_bug.cgi?id=1054941



--- Comment #1 from Michael Schwendt <bugs.michael@xxxxxxx> ---
A brief look only:


> Name:           esteidfirefoxplugin

The modern package Naming Guidelines suggest adding a firefox- prefix, since
this is a plugin that extends Firefox.

  https://fedoraproject.org/wiki/Packaging:Guidelines#Naming
   ->
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28General.29


> Summary:        EstEID browser plugin for digital signing

Is the plugin specific to Firefox? Or is only stored in Firefox's plugin path
but based on the old Netscape Plugin API and then would be compatible with any
browser that supports the NPAPI?

If the summary mentioned Firefox it would be more clear, e.g.

  Summary: Firefox plugin for signing with Estonian ID cards


> License:        LGPLv2

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses


> Requires:       opensc
> Requires:       pcsc-lite
> Requires:       esteidpkcs11loader

It's good practise to add comments to explicit Requires and explain what
exactly is needed. A dependency on a package name could be broken easily, if a
file moves into a different (sub-)package, for example.


> %description
> EstEID Firefox plugin

That's way too short and too lazy for a package description.


> make plugin %{?_smp_mflags}

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

Some packagers like doing something similar to

  %configure || :
  make …

to have the %configure macro export Fedora's CFLAGS and LDFLAGS, if the used
Makefile picks them up.


> %install
> rm -rf %{buildroot}

If you want to target EL5, too, better be explicit about that during package
review because of:
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> %defattr(-,root,root,-)

Not necessary anymore for any of the active dist releases including EL5.


> %doc README.txt 

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text


> %{_libdir}/mozilla/plugins/npesteid-firefox-plugin.so

Please review the File and Directory Ownership section in the guidelines, since
without a dependency on firefox, there would be "unowned" directories.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]