Bug 1548142
| Summary: | Review request: firefox-pkcs11-loader - Helper script for Firefox that sets up the browser for authentication with Estonian ID-card | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Germano Massullo (Thetra) <germano.massullo> |
| Component: | Package Review | Assignee: | Lubomir Rintel <lkundrak> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | lkundrak, package-review, tuju |
| Target Milestone: | --- | Flags: | lkundrak:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2018-11-18 21:44:47 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Germano Massullo (Thetra)
2018-02-22 20:31:52 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" (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 Cool. APPROVED (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/firefox-pkcs11-loader 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 |