Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1224028 - Review Request: rubygem-rmail-sup - A lightweight mail library written in ruby
Summary: Review Request: rubygem-rmail-sup - A lightweight mail library written in ruby
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1024650
TreeView+ depends on / blocked
 
Reported: 2015-05-22 03:51 UTC by Praveen Kumar
Modified: 2015-06-18 13:29 UTC (History)
1 user (show)

Fixed In Version: rubygem-rmail-sup-1.0.1-2.fc22
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-06-18 13:22:01 UTC
Type: ---
Embargoed:
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Praveen Kumar 2015-05-22 03:51:14 UTC
Spec URL: https://kumarpraveen.fedorapeople.org/rubygem-rmail-sup.spec
SRPM URL: https://kumarpraveen.fedorapeople.org/rubygem-rmail-sup-1.0.1-1.fc21.src.rpm
Description: RMail is a lightweight mail library containing various utility classes and modules that allow ruby scripts to parse, modify, and generate MIME mail messages.
Fedora Account System Username:kumarpraveen

$ rpmlint -i rpmbuild/SPECS/rubygem-rmail-sup.spec rpmbuild/SRPMS/rubygem-rmail-sup-1.0.1-1.fc21.src.rpm rpmbuild/RPMS/noarch/rubygem-rmail-sup-1.0.1-1.fc21.noarch.rpm rpmbuild/RPMS/noarch/rubygem-rmail-sup-doc-1.0.1-1.fc21.noarch.rpm 
rubygem-rmail-sup.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

rubygem-rmail-sup-doc.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

3 packages and 1 specfiles checked; 0 errors, 2 warnings.

Comment 1 Parag AN(पराग) 2015-05-22 08:36:27 UTC
Review:

+ mock build is successful for F23 x86_64

+ rpmlint on all the generated rpms gave output
rubygem-rmail-sup.noarch: W: no-documentation
rubygem-rmail-sup-doc.noarch: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

+ Source verified with upstream as sha256sum
upstream tarball: 2f61911c2aa30284c7e2ed3d7bb594a7cb8d20a67774a570a7c0141d40985cf7
tarball in srpm: 2f61911c2aa30284c7e2ed3d7bb594a7cb8d20a67774a570a7c0141d40985cf7

- License in specfile is "GPLv2+" which is invalid. Just check the source files and license is "BSD"

Other suggestions to improve packaging:
1) Group tag is needed for EPEL5 only if package is not supposed to be build on EPEL5 then remove group tag its optional now.

2) defattr(-,root,root,-) is optional now.

3) The Guidelines says "The package must BuildRequires: rubygems-devel to pull in the macros needed to build." 
Add BR: rubygems-devel

4)In %install section as per guildeines I see you missed '/'
mv .%{gem_dir}/* %{buildroot}%{gem_dir}
should be
mv ./%{gem_dir}/* %{buildroot}%{gem_dir}

5) You have written in %install following which I don't see being packaged or no files are getting installed in bindir, good to remove this line
mkdir -p %{buildroot}%{_bindir}

6) I see you only and only need following BR: so remove other BR:
BuildRequires:  rubygems-devel

7) -docs package is installing font files which should be installed separately. Generally we should avoid bundling font files.

8) you can add following to main package
%doc NOTES README NEWS

9) Also check if you can run testsuite in %check as I see test folder in source.


Note we have different ruby packaging guidelines for F19/20, EPEL6/7 and then different guidelines for F21+ releases. 

According to newer guidelines you should follow
a) There should not be any rubygem Requires nor Provides listed, since those are autogenerated.

b) There should not be Requires: ruby(release), unless you want to explicitly specify Ruby version compatibility. Automatically generated dependency on RubyGems (Requires: ruby(rubygems)) is enough.

Comment 2 Praveen Kumar 2015-05-26 06:36:55 UTC
(In reply to Parag AN(पराग) from comment #1)
> Review:
> 
> + mock build is successful for F23 x86_64
> 
> + rpmlint on all the generated rpms gave output
> rubygem-rmail-sup.noarch: W: no-documentation
> rubygem-rmail-sup-doc.noarch: W: no-documentation
> 3 packages and 0 specfiles checked; 0 errors, 2 warnings.
> 
> + Source verified with upstream as sha256sum
> upstream tarball:
> 2f61911c2aa30284c7e2ed3d7bb594a7cb8d20a67774a570a7c0141d40985cf7
> tarball in srpm:
> 2f61911c2aa30284c7e2ed3d7bb594a7cb8d20a67774a570a7c0141d40985cf7
> 
> - License in specfile is "GPLv2+" which is invalid. Just check the source
> files and license is "BSD"
Done
> 
> Other suggestions to improve packaging:
> 1) Group tag is needed for EPEL5 only if package is not supposed to be build
> on EPEL5 then remove group tag its optional now.
Removed Group tag.
> 
> 2) defattr(-,root,root,-) is optional now.
Removed.
> 
> 3) The Guidelines says "The package must BuildRequires: rubygems-devel to
> pull in the macros needed to build." 
> Add BR: rubygems-devel

This was there before also.

> 
> 4)In %install section as per guildeines I see you missed '/'
> mv .%{gem_dir}/* %{buildroot}%{gem_dir}
> should be
> mv ./%{gem_dir}/* %{buildroot}%{gem_dir}

As per %{gem_dir} macro expansion it auto add '/' after '.'
$ rpm --eval %{gem_dir}
/usr/share/gems

> 
> 5) You have written in %install following which I don't see being packaged
> or no files are getting installed in bindir, good to remove this line
> mkdir -p %{buildroot}%{_bindir}

Removed.

> 
> 6) I see you only and only need following BR: so remove other BR:
> BuildRequires:  rubygems-devel

Removed other BRs.

> 
> 7) -docs package is installing font files which should be installed
> separately. Generally we should avoid bundling font files.
> 
> 8) you can add following to main package
> %doc NOTES README NEWS

Added.

> 
> 9) Also check if you can run testsuite in %check as I see test folder in
> source.

Yes but I can't use Rake for run test case as per guideline
[0] https://fedoraproject.org/wiki/Packaging:Ruby?rd=Packaging/Ruby#Running_test_suites 
> 
> 
> Note we have different ruby packaging guidelines for F19/20, EPEL6/7 and
> then different guidelines for F21+ releases. 
> 
> According to newer guidelines you should follow
> a) There should not be any rubygem Requires nor Provides listed, since those
> are autogenerated.
Done
> 
> b) There should not be Requires: ruby(release), unless you want to
> explicitly specify Ruby version compatibility. Automatically generated
> dependency on RubyGems (Requires: ruby(rubygems)) is enough.

Spec URL: https://kumarpraveen.fedorapeople.org/rubygem-rmail-sup.spec
SRPM URL: https://kumarpraveen.fedorapeople.org/rubygem-rmail-sup-1.0.1-2.fc21.src.rpm

Comment 3 Parag AN(पराग) 2015-06-01 06:49:20 UTC
Issues:
1) We don't need to specify explicitly provides: now. See https://fedoraproject.org/wiki/Packaging:Ruby?rd=Packaging/Ruby#Filtering_Requires_and_Provides

Rest looks fine to me.

Comment 4 Praveen Kumar 2015-06-03 05:16:19 UTC
New Package SCM Request
=======================
Package Name: rubygem-rmail-sup
Short Description: A lightweight mail library written in ruby
Upstream URL: http://sup.rubyforge.org/
Owners: kumarpraveen
Branches: f20 f21 f22
InitialCC: shreyankg

Comment 5 Gwyn Ciesla 2015-06-03 12:44:23 UTC
f20 is no longer accepting new packages, and InitialCC needs a FAS account
name, not an email address.

Comment 6 Praveen Kumar 2015-06-04 03:12:48 UTC

New Package SCM Request
=======================
Package Name: rubygem-rmail-sup
Short Description: A lightweight mail library written in ruby
Upstream URL: http://sup.rubyforge.org/
Owners: kumarpraveen
Branches: f21 f22
InitialCC: shreyankg

Comment 7 Gwyn Ciesla 2015-06-04 13:09:36 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2015-06-05 07:26:13 UTC
rubygem-rmail-sup-1.0.1-2.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/rubygem-rmail-sup-1.0.1-2.fc22

Comment 9 Fedora Update System 2015-06-05 08:14:55 UTC
rubygem-rmail-sup-1.0.1-2.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/rubygem-rmail-sup-1.0.1-2.fc21

Comment 10 Fedora Update System 2015-06-07 16:05:43 UTC
rubygem-rmail-sup-1.0.1-2.fc21 has been pushed to the Fedora 21 testing repository.

Comment 11 Fedora Update System 2015-06-18 13:22:01 UTC
rubygem-rmail-sup-1.0.1-2.fc21 has been pushed to the Fedora 21 stable repository.

Comment 12 Fedora Update System 2015-06-18 13:29:56 UTC
rubygem-rmail-sup-1.0.1-2.fc22 has been pushed to the Fedora 22 stable repository.


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