Bug 1548142 - Review request: firefox-pkcs11-loader - Helper script for Firefox that sets up the browser for authentication with Estonian ID-card
Summary: Review request: firefox-pkcs11-loader - Helper script for Firefox that sets u...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-02-22 20:31 UTC by Germano Massullo (Thetra)
Modified: 2019-08-27 08:50 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-11-18 21:44:47 UTC
Type: Bug
Embargoed:
lkundrak: fedora-review+


Attachments (Terms of Use)

Description Germano Massullo (Thetra) 2018-02-22 20:31:52 UTC
Helper script for Firefox that sets up the browser for authentication with Estonian ID-card

https://germano.fedorapeople.org/package_reviews/firefox-pkcs11-loader/firefox-pkcs11-loader.spec

https://germano.fedorapeople.org/package_reviews/firefox-pkcs11-loader/firefox-pkcs11-loader-3.13.0-1.fc27.src.rpm

package will unretire firefox-esteidpkcs11loader and change its name to firefox-pkcs11-loader in order to be equal to upstream as much as possible

Procedure for package renaming is https://fedoraproject.org/wiki/Package_Renaming_Process

Comment 1 Lubomir Rintel 2018-02-22 20:45:16 UTC
0.) Please remove a useless comment:

> # procedure for package renaming is https://fedoraproject.org/wiki/Package_Renaming_Process

1.) Please drop the useless macro:

> %global source_name firefox-pkcs11-loader

Use %{name} in place of %{source_name}

2.) Don't mix tabs and spaces:
   
> # old name was firefox-esteidpkcs11loader
> Obsoletes:              firefox-esteidpkcs11loader
            ^^^^^^^^^^^^^^
            Please replace this with spaces.

3.) Please use macros consistentlt:

> %dir %{_prefix}/lib/mozilla/pkcs11-modules/
> %_prefix/lib/mozilla/pkcs11-modules/onepinopenscpkcs11.json
  ^^^^^^^^
  %{_prefix} here

4.) Missing dependency on firefox-filesystem

You install files into %{_prefix}/lib/mozilla and
%{_datadir}/mozilla/extensions that are owned by it

5.) Summary is too long

> Summary:        Helper script for Firefox that sets up the browser for authentication with Estonian ID-card

And doesn't seem to make sense -- what package ships is not a script.
A better choice would be something like:
"Estonian ID-card authentication support for Firefox"

Comment 2 Germano Massullo (Thetra) 2018-02-22 21:19:13 UTC
(In reply to Lubomir Rintel from comment #1)
> 0.) Please remove a useless comment:
> 
> > # procedure for package renaming is https://fedoraproject.org/wiki/Package_Renaming_Process

Done

> 1.) Please drop the useless macro:
> 
> > %global source_name firefox-pkcs11-loader
> 
> Use %{name} in place of %{source_name}

Done

> 2.) Don't mix tabs and spaces:
>    
> > # old name was firefox-esteidpkcs11loader
> > Obsoletes:              firefox-esteidpkcs11loader
>             ^^^^^^^^^^^^^^
>             Please replace this with spaces.

Done

> 3.) Please use macros consistentlt:
> 
> > %dir %{_prefix}/lib/mozilla/pkcs11-modules/
> > %_prefix/lib/mozilla/pkcs11-modules/onepinopenscpkcs11.json
>   ^^^^^^^^
>   %{_prefix} here

Done

> 4.) Missing dependency on firefox-filesystem
> 
> You install files into %{_prefix}/lib/mozilla and
> %{_datadir}/mozilla/extensions that are owned by it

Done

> 5.) Summary is too long
> 
> > Summary:        Helper script for Firefox that sets up the browser for authentication with Estonian ID-card


There is no requirement for 80 char length on summary, there is only on description
https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description


> And doesn't seem to make sense -- what package ships is not a script.
> A better choice would be something like:
> "Estonian ID-card authentication support for Firefox"

it is the same description from upstream
https://github.com/open-eid/firefox-pkcs11-loader
and it is a package that installs a JSON and an extension that configures Firefox to correctly handle PKCS11. This configuration is required to use correctly the card, but you could also configure manually Firefox instead of installing this package

Comment 3 Lubomir Rintel 2018-02-22 21:22:05 UTC
Cool.

APPROVED

Comment 4 Gwyn Ciesla 2018-02-23 01:02:49 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/firefox-pkcs11-loader

Comment 5 Fedora Update System 2018-03-06 16:50:05 UTC
firefox-pkcs11-loader-3.13.0-3.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-bb58637f4c


Note You need to log in before you can comment on or make changes to this bug.