Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1063060 - Review Request: rubygem-websocket - Universal Ruby library to handle WebSocket protocol
Summary: Review Request: rubygem-websocket - Universal Ruby library to handle WebSocke...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1060174
TreeView+ depends on / blocked
 
Reported: 2014-02-09 18:23 UTC by Nitesh Narayan Lal
Modified: 2015-07-21 12:48 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-06-08 18:16:52 UTC
vondruch: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Nitesh Narayan Lal 2014-02-09 18:23:06 UTC
Spec URL: http://niteshnarayan.fedorapeople.org/SPECS/rubygem-websocket-1.1.2.spec
SRPM URL: http://niteshnarayan.fedorapeople.org/SRPMS/rubygem-websocket-1.1.2-1.fc19.src.rpm
Description: Universal Ruby library to handle WebSocket protocol
Fedora Account System Username:
niteshnarayan

Comment 1 Mo Morsi 2015-04-24 23:56:36 UTC
Hey nitesh, wanted to confirm you expressed disinterest in seeing the packages you submitted not yet reviewed correct? As per private communications and https://bugzilla.redhat.com/show_bug.cgi?id=1063048

If this is not the case please respond asap.

Taking this submission over this for the time being as the broken rubygem-selenium-webdriver depends on this.

Updated new package:

Spec: https://mmorsi.fedorapeople.org/staging/rubygem-websocket-1.2.2.spec
SRPM: https://mmorsi.fedorapeople.org/staging/rubygem-websocket-1.2.2-1.fc21.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=9562840

Comment 2 Vít Ondruch 2015-04-30 08:32:09 UTC
I'll take this for a review.

Comment 3 Vít Ondruch 2015-04-30 08:43:51 UTC
* .spec file naming.
  - Please remove the version from the .spec file name, since per
    guidelines [1], the .spec file naming should be "%{name}.spec"

* test suite
  - Please execute the test suite. Executing "spec lib" is probably typo.
  - It would be nice if you can move the %check section below %install section.
    This is how typical .spec file looks and it nicely follows rpmbuild's
    execution sequence.

* -doc subpackage content
  - Please consider to move non-substantial stuff into -doc subpackage. I.e. 

    %{gem_instdir}/Gemfile
    %{gem_instdir}/Rakefile
    %{gem_instdir}/spec/*
    %{gem_instdir}/websocket.gemspec

    files should be probably in -doc subpackage.

* License file
  - Since there is no separate LICENSE file, you should

    1) ask upstream to include such file.
    2) move README.md into the main package, since it contains license
       information.

* Missing period at the end of description.
  - Description should end by period.


Please fix these issues prior I continue with the review.



[1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines?rd=Packaging/NamingGuidelines#Spec_file_name

Comment 4 Nitesh Narayan Lal 2015-04-30 14:52:45 UTC
It seems, I won't be able to give time to this due to may day job.
I am sorry for the delay in updating the status.
If Mo could take this up then its great, otherwise I will close it.

Comment 5 Mo Morsi 2015-05-01 16:03:32 UTC
Updated. Don't worry about it Nitesh Vit/I have this one.

Spec: https://mmorsi.fedorapeople.org/staging/rubygem-websocket.spec
SRPM: https://mmorsi.fedorapeople.org/staging/rubygem-websocket-1.2.2-2.fc21.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=9617517

The rubygem-websocket spec suite depends on the "its" gem which isn't in Fedora yet, so left that commented for now.

https://github.com/imanel/websocket-ruby/blob/master/spec/spec_helper.rb#L2

Comment 6 Vít Ondruch 2015-05-07 12:03:49 UTC
(In reply to Mo Morsi from comment #5)
> The rubygem-websocket spec suite depends on the "its" gem which isn't in
> Fedora yet, so left that commented for now.

Better part of the test suite can be enabled as simple as:

find spec -name *.rb | xargs sed -i '/its/ s/^/#/'

BTW if you keep the test suite disabled, the BR: rubygem(rspec) is worthless.

* Superfluous requires
  - The requires are not auto-generated during build. Please remove them.

* Missing period after description.
  - The -doc subpackage description should end by period.

* Mark documentation by %doc macro
  - The CHANGELOG.md is documentation and hence it should be marked by %doc
    macro.(In reply to Mo Morsi from comment #5)

* Unowned directory
  - I believe that the %{gem_instdir}/spec/* makes the RPM to own just the
    content of the directory, not the directory itself.


The above points are just minor nits and the package otherwise looks good => APPROVED but please fix the issues prior importing the package into Fedora.

Comment 8 Mo Morsi 2015-05-07 19:46:41 UTC
Package Change Request
======================
Package Name: rubygem-websocket
New Branches: f22 f21
Owners: mmorsi

Comment 9 Mo Morsi 2015-05-07 19:49:13 UTC
Sorry copy-n-pasted wrong template

New Package SCM Request
=======================
Package Name: rubygem-webmock
Short Description: Universal Ruby library to handle WebSocket protocol
Upstream URL: http://rubygems.org/gems/webmock
Owners: mmorsi
Branches:
InitialCC:

Comment 10 Gwyn Ciesla 2015-05-08 16:56:54 UTC
WARNING: Requested package name rubygem-webmock doesn't match bug summary
rubygem-websocket

Comment 11 Mo Morsi 2015-05-08 20:44:51 UTC
Sorry too many similarly named packages

New Package SCM Request
=======================
Package Name: rubygem-websocket
Short Description: Universal Ruby library to handle WebSocket protocol
Upstream URL: http://rubygems.org/gems/websocket
Owners: mmorsi
Branches:
InitialCC:

Comment 12 Gwyn Ciesla 2015-05-08 22:28:58 UTC
Git done (by process-git-requests).

Comment 13 Mo Morsi 2015-06-08 18:16:52 UTC
rubygem-websocket has been pushed to / built against rawhide

http://koji.fedoraproject.org/koji/buildinfo?buildID=636957


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