Bug 1063060 - Review Request: rubygem-websocket - Universal Ruby library to handle WebSocket protocol
Review Request: rubygem-websocket - Universal Ruby library to handle WebSocke...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Vít Ondruch
Fedora Extras Quality Assurance
Depends On:
Blocks: 1060174
  Show dependency treegraph
Reported: 2014-02-09 13:23 EST by Nitesh Narayan Lal
Modified: 2015-07-21 08:48 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2015-06-08 14:16:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
vondruch: fedora‑review+
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Nitesh Narayan Lal 2014-02-09 13:23:06 EST
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:
Comment 1 Mo Morsi 2015-04-24 19:56:36 EDT
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 04:32:09 EDT
I'll take this for a review.
Comment 3 Vít Ondruch 2015-04-30 04:43:51 EDT
* .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. 


    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

* 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 10:52:45 EDT
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 12:03:32 EDT
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.

Comment 6 Vít Ondruch 2015-05-07 08:03:49 EDT
(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 15:46:41 EDT
Package Change Request
Package Name: rubygem-websocket
New Branches: f22 f21
Owners: mmorsi
Comment 9 Mo Morsi 2015-05-07 15:49:13 EDT
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
Comment 10 Gwyn Ciesla 2015-05-08 12:56:54 EDT
WARNING: Requested package name rubygem-webmock doesn't match bug summary
Comment 11 Mo Morsi 2015-05-08 16:44:51 EDT
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
Comment 12 Gwyn Ciesla 2015-05-08 18:28:58 EDT
Git done (by process-git-requests).
Comment 13 Mo Morsi 2015-06-08 14:16:52 EDT
rubygem-websocket has been pushed to / built against rawhide


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