Bug 949317 - Review Request: rubygem-serialport - Ruby library that provide class for RS232 serail port
Summary: Review Request: rubygem-serialport - Ruby library that provide class for RS23...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Luis Bazan
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-04-07 17:13 UTC by Alejandro_Perez
Modified: 2014-03-15 15:11 UTC (History)
3 users (show)

Fixed In Version: rubygem-serialport-1.3.0-5.fc20
Clone Of:
Environment:
Last Closed: 2014-03-15 15:11:31 UTC
Type: ---
Embargoed:
bazanluis20: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Alejandro_Perez 2013-04-07 17:13:36 UTC
Spec URL: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport.spec
SRPM URL: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport-1.1.0-1.fc19.src.rpm 
Description: Ruby SerialPort is a class for using RS232 serial ports. It also contains low-level function to check current state of signals on the line.
Fedora Account System Username:aeperezt

Comment 1 Alejandro_Perez 2013-04-07 17:21:17 UTC
rpmlint Output:

rpmlint SPECS/rubygem-serialport.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint SRPMS/rubygem-serialport-1.1.0-1.fc19.src.rpm 
rubygem-serialport.src: W: strange-permission rubygem-serialport.spec 0600L
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
rpmlint RPMS/x86_64/rubygem-serialport-1.1.0-1.fc19.x86_64.rpm 
rubygem-serialport.x86_64: W: no-soname /usr/lib64/gems/ruby/serialport-1.1.0/lib/serialport.so
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Comment 2 Eduardo Echeverria 2013-06-11 07:31:45 UTC
There are some issues in fedora-review which appear to be false positives
- Pure Ruby package must be built as noarch
  See: https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support
this package contains C extensions, therefore are dependent arch

if you don't want ship the package to el5:
- %clean is not needed
- BuildRoot is not needed
- cleaning of buildroot in %install is not needed
- %defattr is not needed

- Compiler flags do not honor fedora specific. please add 
export CONFIGURE_ARGS="--with-cflags='%{optflags}'"
above %gem_install

- Please  exclude cached Gem with %exclude %{gem_cache}
- There are tests in the package, should be executed
- Add the license file, exists in the source.
- the files README CHANGELOG are twice listed, please fix it

Comment 3 Vít Ondruch 2013-06-13 11:44:11 UTC
(In reply to Eduardo Echeverria from comment #2)
> There are some issues in fedora-review which appear to be false positives
> - Pure Ruby package must be built as noarch
>   See:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support
> this package contains C extensions, therefore are dependent arch

The package is not marked as noarch as far as I can say, but the -doc subpackage should be noarch IMO.

BTW why is there "ExcludeArch:   ppc ppc64"? Is that known that these package does not work on PPC? Sorry, I did not checked, I'm just wondering.

> - Compiler flags do not honor fedora specific. please add 
> export CONFIGURE_ARGS="--with-cflags='%{optflags}'"
> above %gem_install

The CONFIGURE_ARGS variable now part of %gem_install macro, so it is OK to be omitted.

* Please use %{gem_instdir} in place of %{gem_dir}/gems/%{gem_name}-%{version}/
  - We have macro for this construct, no need to be so verbose
  - Moreover, we use to prepend %dir to this macro and include the content of
    this directory explicitly. Although it is more work, it gives you a bit more
    fine grained control during updates, what goes into package and what was
    changed, etc.

Comment 4 Alejandro_Perez 2013-08-22 06:37:04 UTC
I added the ExcludeArch to be safe had not tested on those plataforms, removed from the spec file, added recomended changes.

Spec: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport.spec
SRPM: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport-1.1.0-3.fc19.src.rpm

Comment 5 Vít Ondruch 2013-08-22 07:51:24 UTC
* Separate changelog entries by empty line
  - It is common
  - It is easier to read
  - Some tools cannot handle the changelog properly. Try "$ fedpkg clog" for
    example.

* Remove BuildRoot
  - This tag is not needed for Fedora. It is useful just for EPEL5.

* Remove %defattr
  - This is now default. No need to specify. Please remove the macro.

* Exclude %{gem_cache}
  - Please %exclude %{gem_cache}. This file has no purpose on Fedora.

* Please execute test suite if possible.

* Backslash in %description is not needed IMO.

* %{gem_instdir} ownership
  - I typically suggest to structure %files section as follows
    
    %dir %{gem_instdir}
    %{gem_libdir}

  - You own directly the %{gem_instdir}, but that means you own whole content
    of that directory. It means that during further updates, there might slip
    through your hands important additions etc. This is not a show stopper, but
    good practice IMO.

Comment 6 Alejandro_Perez 2013-08-22 14:21:26 UTC
Added the changes.
About the test not sure how to execute the test, this test is to use the serial port so to execute the test we need to give serial port device , bauds and all that. So we cannot run it or at least I don't know how to do it.

Spec: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport.spec
SRPM: http://aeperezt.fedorapeople.org/rpmdev/rubygem-serialport-1.1.0-4.fc19.src.rpm
Thanks for you help.

Comment 7 Alejandro_Perez 2013-12-28 02:26:38 UTC
Koji Rawhide build http://koji.fedoraproject.org/koji/taskinfo?taskID=6337956

Comment 8 Alejandro_Perez 2013-12-28 02:29:09 UTC
Sorry wrong koji link, this one is correct http://koji.fedoraproject.org/koji/taskinfo?taskID=6337919

Comment 10 Luis Bazan 2014-03-13 15:12:51 UTC
Hi Alejandro

You have met all the requested changes.

I just did one last review and  all comply with the guidelines.

Approved!

Best Regards!

Comment 11 Alejandro_Perez 2014-03-13 16:25:49 UTC
New Package SCM Request
=======================
Package Name: Rubygem-serialport
Short Description:  Ruby class for using RS232 serial ports
Owners: aeperezt
Branches: f19,f20 el6 epel7

Comment 12 Alejandro_Perez 2014-03-13 16:30:47 UTC
Change the name to lower case. Sorry for that.

New Package SCM Request
=======================
Package Name: rubygem-serialport
Short Description:  Ruby class for using RS232 serial ports
Owners: aeperezt
Branches: f19,f20 el6 epel7

Comment 13 Alejandro_Perez 2014-03-13 16:42:57 UTC
Update the request to remove coma

New Package SCM Request
=======================
Package Name: rubygem-serialport
Short Description:  Ruby class for using RS232 serial ports
Owners: aeperezt
Branches: f19 f20 el6 epel7

Comment 14 Gwyn Ciesla 2014-03-13 17:31:18 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2014-03-13 20:34:12 UTC
rubygem-serialport-1.3.0-5.fc19,rubygem-icaro-1.0.6-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/rubygem-serialport-1.3.0-5.fc19,rubygem-icaro-1.0.6-2.fc19

Comment 16 Fedora Update System 2014-03-13 20:34:27 UTC
rubygem-serialport-1.3.0-5.fc20,rubygem-icaro-1.0.6-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/rubygem-serialport-1.3.0-5.fc20,rubygem-icaro-1.0.6-2.fc20

Comment 17 Fedora Update System 2014-03-15 15:11:31 UTC
rubygem-serialport-1.3.0-5.fc19, rubygem-icaro-1.0.6-2.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2014-03-15 15:11:57 UTC
rubygem-serialport-1.3.0-5.fc20, rubygem-icaro-1.0.6-2.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.


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