Bug 976468
Summary: | Review Request: rubygem-teamocil - Easy session, window and pane layouts for tmux | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Adam Miller <admiller> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED DEFERRED | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | marcelo.barbosa, notting, package-review, vondruch |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2013-07-31 15:23:34 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
Adam Miller
2013-06-20 15:51:24 UTC
Hi Adam, I'm reviewing your package, please see this issues: Issues: ======= - gems should require rubygems package Note: Requires: rubygems missing in rubygem-teamocil-doc See: http://fedoraproject.org/wiki/Packaging:Ruby#RubyGems - Package does not contain duplicates in %files. Note: warning: File listed twice: /usr/share/gems/gems/teamocil-0.4.4 See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles - If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. Note: Cannot find LICENSE in rpm(s) See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text Thank you. Marcelo Barbosa (In reply to Marcelo Barbosa from comment #1) > Hi Adam, > > I'm reviewing your package, please see this issues: > > Issues: > ======= > - gems should require rubygems package > Note: Requires: rubygems missing in rubygem-teamocil-doc > See: http://fedoraproject.org/wiki/Packaging:Ruby#RubyGems The -doc subpackage is requiring the main package, which requires ruby(rubygems), so this should be OK IMO. > - Package does not contain duplicates in %files. > Note: warning: File listed twice: /usr/share/gems/gems/teamocil-0.4.4 > See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles Yes, the %{gem_instdir} (without the %dir macro) should be removed. * Use %gem_install macro - Could you please use %gem_install as per latest Ruby packaging guidelines [1]. - Please note that latest gem2rpm supports this macro out of the box. You should use "-t fedora-19-rawhide" template specifier though. * Use ruby(release) instead of ruby(abi) - Latest guidelines requires you to use ruby(release) virtual provide instead of ruby(abi) [2]. Please update your .spec file appropriately. [1] https://fedoraproject.org/wiki/Packaging:Ruby#Building_gems [2] https://fedoraproject.org/wiki/Packaging:Ruby#Ruby_Compatibility I've updated and taken into account the feedback, rpmlint passed on my F18 machine but I would like to point out that the latest ruby guidelines for using ruby(release) don't work on Fedora 18: Getting requirements for rubygem-teamocil-0.4.4-2.fc18.src --> rubygems-devel-1.8.25-6.fc18.noarch --> ruby-1.9.3.429-30.fc18.x86_64 Error: No Package found for ruby(release) This did however build just fine for F19, what's the recommendations for F18? Is the ruby(release) functionality expected to land in F18 at any point or will this be something I'll need to maintain two different spec files for right out of the gates? Spec URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil.spec SRPM URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil-0.4.4-2.fc19.src.rpm No, ruby(release) will never be backported. For F18 and older releases, we stay with ruby(abi). You should either maintain two versions of .spec file (that is preferred) or conditionalize the build. BTW your %files section still contains: %dir %{gem_instdir} %{gem_instdir} Which is wrong. I would suggest to keep the %dir prefixed entry and fix the rest of issues, which it will cause. Hi Adam, I ran this first check in your new package: $ fedora-review -b 976468 -m fedora-hawhide-x86_64 And this are outputs issues for your new package: Issues: ======= - Package contains Requires: ruby(abi). See: http://fedoraproject.org/wiki/Packaging:Ruby#Ruby_ABI - Package does not contain duplicates in %files. Note: warning: File listed twice: /usr/share/gems/gems/teamocil-0.4.4 See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles - If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. Note: Cannot find LICENSE in rpm(s) See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text Please adjust this problems, and generate new package. Thank you. Marcelo Barbosa (In reply to Marcelo Barbosa from comment #5) > - Package contains Requires: ruby(abi). > See: http://fedoraproject.org/wiki/Packaging:Ruby#Ruby_ABI Please don't blindly believe to Fedora Review, since there have not been merged changes [1] needed by recent guidelines [2]. [1] https://fedorahosted.org/FedoraReview/ticket/212 [2] https://fedoraproject.org/wiki/Packaging:Ruby#Ruby_Compatibility Vit, I followed your advise, I think I misunderstood at first but I went back over the guidelines for F19 ruby packaging and I think I've got it sorted out now. Spec URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil.spec SRPM URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil-0.4.4-3.fc19.src.rpm * Consider to package this gem as an Application - I am not sure if this gem has any meaning as a gem, i.e. if it can be used Ruby library. Have you considered the possibility to package it as an application [1]? * Please execute test suite - The gem ships with test suite, could you please execute it during build? * Exclude hidden files - Although you exclude .gitignore file, you keep .travis.yml in package. I would suggest to exclude both using %exclude %{gem_instdir}/.* * Move files not needed for runtime into -doc subpackage - Please consider move of examples, spec, Gemfile, README.md and Rakefile into -doc subpackage, since they are not needed for runtime. [1] https://fedoraproject.org/wiki/Packaging:Ruby#Applications I'm closing this, I've switched to tmuxinator https://github.com/aziz/tmuxinator and will open a review request for this soon. |