Bug 866982

Summary: Review Request: rubygem-gem-patch - RubyGems plugin for patching gems.
Product: [Fedora] Fedora Reporter: Josef Stribny <jstribny>
Component: Package ReviewAssignee: Vít Ondruch <vondruch>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: hhorak, notting, package-review, vondruch
Target Milestone: ---Flags: vondruch: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-11-16 02:35:56 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Josef Stribny 2012-10-16 09:30:57 EDT
Spec URL: http://data-strzibny.rhcloud.com/rubygem-gem-patch.spec
SRPM URL: rubygem-gem-patch-0.1.2-1.fc17.src.rpm
Description: `gem-patch` is a RubyGems plugin that helps to patch gems without manually opening and rebuilding them. It opens a given .gem file, extracts it, patches it with system `patch` command, clones its spec, updates the file list and builds the patched gem.
Fedora Account System Username: jstribny
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4594605

This is my first package for Fedora, so I need a sponsor. I am also the upstream maintainer.
Comment 1 Josef Stribny 2012-10-16 09:41:09 EDT
SRPM full URL:

http://data-strzibny.rhcloud.com/rubygem-gem-patch-0.1.2-1.fc17.src.rpm
Comment 2 Vít Ondruch 2012-10-16 10:56:27 EDT
Hi,

I'll take this package for a review and I can sponsor you later as well. Please follow the procedures at [1] as well as [2].


[1] https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
[2] https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Comment 3 Vít Ondruch 2012-10-17 07:19:59 EDT
First of all, it seems you created the .spec file by hand. While that is perfectly fine, gem2rpm could save you some work.

* Source URL
  - You did not provided full source URL. Your URL should be:

    Source0: http://rubygems.org/gems/%{gem_name}-%{version}.gem

* %description 80 chars wrap
  - In %description, there should be no lines longer than 80 characters [1].

* Missing %changelog
  - Please add %changelog section into your .spec file [2].

* -doc subpackage
  - Please consider to move documentation into -doc subpackage [3].


[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description
[2] https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
[3] https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#PackageDocumentation
Comment 4 Josef Stribny 2012-10-18 06:44:12 EDT
Thanks Vít,

I packed it using gem2rpm and fixed the mentioned issues on the way.

Resulting spec file and SRPM are located on the same URLs as before [1, 2].
I am also including a link to koji results [3] for `koji build --scratch f19`.

[1] http://data-strzibny.rhcloud.com/rubygem-gem-patch.spec
[2] http://data-strzibny.rhcloud.com/rubygem-gem-patch-0.1.2-1.fc17.src.rpm
[3] http://koji.fedoraproject.org/koji/taskinfo?taskID=4602940
Comment 5 Vít Ondruch 2012-10-19 08:51:57 EDT
* Update release number between release
  - You should bump the release number between each iteration of review.
    It helps to keep track of changes. You keep a record of changes in
    changelog as well.

* Description wrapping
  - Isn't it strangely wrapped? Hint: single word on line.

* Keep tests in -doc subpackage
  - We do not through out the test suite if it is part of the package. However,
    if there is -doc subpackage, the test suite should be moved there. 
    See "MUST: Large documentation files must go in a -doc subpackage.
    (The definition of large is left up to the packager's best judgement, but
    is not restricted to size. Large can refer to either size or quantity)."
    in [1].

* Keep the license in main package
  - See "MUST: 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 must be included in %doc." in [1].

* Move rakefile.rb into -doc subpackage
  - Since this file is not required for runtime, I suggest to move it into -doc
    subpackage.




[1] http://http://fedoraproject.org/wiki/Packaging:ReviewGuidelines
Comment 6 Josef Stribny 2012-10-22 11:30:00 EDT
Thanks,

I fixed the issues you listed and also upgraded to gem-patch-0.1.3:

SPEC: http://data-strzibny.rhcloud.com/rubygem-gem-patch.spec
SRPM: http://data-strzibny.rhcloud.com/rubygem-gem-patch-0.1.3-1.fc17.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4615807
Comment 7 Vít Ondruch 2012-10-23 09:39:45 EDT
The .spec you link here differs from spec in your SRPM. Which one is correct? Could you please fix this issue? Thank you.
Comment 8 Josef Stribny 2012-10-25 03:10:24 EDT
Hi,

I uploaded the corresponding SRPM, it wasn't up-to-date. Apologies.

SPEC: http://data-strzibny.rhcloud.com/rubygem-gem-patch.spec
SRPM: http://data-strzibny.rhcloud.com/rubygem-gem-patch-0.1.3-1.fc17.src.rpm
Comment 9 Vít Ondruch 2012-10-25 06:44:59 EDT
* pushd paired with popd
  - This is minor nit and has no influence on anything, but these are pair
    commands and they should be used in pair IMO (see %check section).

BTW Upstream question: What is the purpose of two identical READMEs?

Otherwise the package looks good => APPROVED.

Since you proven that you understand the guidelines by passing two packages through review and poking around for some informal reviews, I am going to sponsor you as well. Enjoy your new privileges for good of community ;)
Comment 11 Josef Stribny 2012-10-29 05:11:33 EDT
New Package SCM Request
=======================
Package Name: rubygem-gem-patch
Short Description: RubyGems plugin for patching gems
Owners: jstribny
Branches: f17 f18
InitialCC:
Comment 12 Gwyn Ciesla 2012-10-29 09:11:26 EDT
Git done (by process-git-requests).
Comment 13 Fedora Update System 2012-10-29 10:34:22 EDT
rubygem-gem-patch-0.1.3-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/rubygem-gem-patch-0.1.3-1.fc18
Comment 14 Fedora Update System 2012-10-29 10:37:59 EDT
rubygem-gem-patch-0.1.3-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/rubygem-gem-patch-0.1.3-1.fc17
Comment 15 Fedora Update System 2012-10-29 14:12:47 EDT
rubygem-gem-patch-0.1.3-1.fc18 has been pushed to the Fedora 18 testing repository.
Comment 16 Fedora Update System 2012-11-16 02:35:58 EST
rubygem-gem-patch-0.1.3-1.fc17 has been pushed to the Fedora 17 stable repository.
Comment 17 Fedora Update System 2012-12-01 04:52:25 EST
rubygem-gem-patch-0.1.3-1.fc18 has been pushed to the Fedora 18 stable repository.