Bug 976468 - Review Request: rubygem-teamocil - Easy session, window and pane layouts for tmux
Review Request: rubygem-teamocil - Easy session, window and pane layouts for ...
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-20 11:51 EDT by Adam Miller
Modified: 2016-05-15 09:38 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-07-31 11:23:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Adam Miller 2013-06-20 11:51:24 EDT
Spec URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil.spec
SRPM URL: http://maxamillion.fedorapeople.org/rubygem-teamocil/rubygem-teamocil-0.4.4-1.fc18.src.rpm
Description: Teamocil helps you set up window and pane layouts for tmux using YAML configuration files.
Fedora Account System Username: maxamillion
Comment 1 Marcelo Barbosa "firemanxbr" 2013-06-20 15:20:03 EDT
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
Comment 2 Vít Ondruch 2013-06-27 04:40:13 EDT
(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
Comment 3 Adam Miller 2013-06-27 10:21:09 EDT
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
Comment 4 Vít Ondruch 2013-06-28 04:29:29 EDT
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.
Comment 5 Marcelo Barbosa "firemanxbr" 2013-07-01 00:01:19 EDT
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
Comment 6 Vít Ondruch 2013-07-01 03:23:19 EDT
(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
Comment 7 Adam Miller 2013-07-02 14:17:46 EDT
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
Comment 8 Vít Ondruch 2013-07-09 02:38:02 EDT
* 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
Comment 9 Adam Miller 2013-07-31 11:23:34 EDT
I'm closing this, I've switched to tmuxinator https://github.com/aziz/tmuxinator and will open a review request for this soon.

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