Bug 989775
| Summary: | Review Request: rubygem-omniauth - A generalized Rack framework for multiple-provider authentication | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Achilleas Pipinellis <axilleas> |
| Component: | Package Review | Assignee: | Lubomir Rintel <lkundrak> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | lkundrak, notting |
| Target Milestone: | --- | Flags: | lkundrak:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | rubygem-omniauth-1.1.4-2.fc20 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2013-10-02 06:27:07 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Achilleas Pipinellis
2013-07-29 21:52:51 UTC
* The package is properly named
* Versioning is sane
* License is fine
* SPEC file is clean and legible
* RPMlint is happy
* Builds fine in mock
* Source file matches upstream
* Requires sane
* Provides sane
* Filelist sane
1.) No need to explicitely set noarch here
%package doc
...
BuildArch: noarch
2.) It's probably not a good idea to modify the files in build tree from check.
%prep is better suited for this purpose:
%check
pushd .%{gem_instdir}
# Remove unneeded coveralls
sed -i '/[Cc]overalls/d' spec/helper.rb
3.) Keeping README in -doc subpackage is a bad idea
...despite what some Ruby packagers unaware of RPM transaction flags think :(
Please move it into the main package. User should not need to install a
subpackage to learn important information about the main package.
%files doc
...
%doc %{gem_instdir}/README.md
Thanks for the review :) All fixed. SPEC: http://axilleas.fedorapeople.org/pkgs/rubygem-omniauth/rubygem-omniauth.spec SRPM: http://axilleas.fedorapeople.org/pkgs/rubygem-omniauth/rubygem-omniauth-1.1.4-2.fc19.src.rpm New koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5859513 Also could you elaborate on the README and the `RPM transaction flags` you mentioned? Thank you :) (In reply to Axilleas Pipinellis from comment #3) > Also could you elaborate on the README and the `RPM transaction flags` you > mentioned? Thank you :) See "rpm --excludedocs" and "tsflags" in yum.conf(5). Thank you for addressing the issues, the package looks fine to me now. APPROVED Sorry for the late reply, thanks for the review :) New Package SCM Request ======================= Package Name: rubygem-omniauth Short Description: A generalized Rack framework for multiple-provider authentication Owners: axilleas Branches: f19 f20 InitialCC: Git done (by process-git-requests). rubygem-omniauth-1.1.4-2.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/rubygem-omniauth-1.1.4-2.fc20 rubygem-omniauth-1.1.4-2.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/rubygem-omniauth-1.1.4-2.fc19 rubygem-omniauth-1.1.4-2.fc20 has been pushed to the Fedora 20 testing repository. rubygem-omniauth-1.1.4-2.fc19 has been pushed to the Fedora 19 stable repository. rubygem-omniauth-1.1.4-2.fc20 has been pushed to the Fedora 20 stable repository. |