[Bug 1224028] Review Request: rubygem-rmail-sup - A lightweight mail library written in ruby

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1224028



--- Comment #2 from Praveen Kumar <kumarpraveen.nitdgp@xxxxxxxxx> ---
(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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]