[Bug 1667935] New: Review request nodejs-mqtt - MQTT client library for nodejs

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

Bug ID: 1667935
           Summary: Review request nodejs-mqtt - MQTT client library for
                    nodejs
           Product: Fedora
           Version: rawhide
            Status: NEW
         Component: Package Review
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL: https://fedorapeople.org/~humaton/rpms/new/nodejs-mqtt.spec
SRPM URL:
https://fedorapeople.org/~humaton/rpms/new/nodejs-mqtt-2.18.8-1.fc29.src.rpm

Description: MQTT.js is a client library for the MQTT protocol, written in
JavaScript for node.js and the browser.

Fedora Account System Username: humaton

[Bug 1667935] Review request nodejs-mqtt - MQTT client library for nodejs

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

Hirotaka Wakabayashi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Hirotaka Wakabayashi  ---
Hello, this is an unofficial review. Please read this for your reference.

Summary
=======

1. rpmlint results
2. The 'Group' tag should not be used
3. Other suggestions
4. Appendix 2: Koji scratch build succeeded

Details
=======

1. rpmlint results
------------------

A warning on the source rpm and 24 warnings on the binary rpm, which
I built on my fc29 environment::
  $ rpmlint /home/vagrant/rpmbuild/SRPMS/nodejs-mqtt-2.18.8-1.fc29.src.rpm
  nodejs-mqtt.src: W: no-%build-section
  1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint
/home/vagrant/rpmbuild/RPMS/noarch/nodejs-mqtt-2.18.8-1.fc29.noarch.rpm
  nodejs-mqtt.noarch: W: incoherent-version-in-changelog 2.18.8-2
['2.18.8-1.fc29', '2.18.8-1']
  nodejs-mqtt.noarch: W: only-non-binary-in-usr-lib
  nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/examples/tls client/crt.ca.cg.pem
  nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/examples/tls client/tls-cert.pem
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/commist /usr/lib/node_modules/commist
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/concat-stream
/usr/lib/node_modules/concat-stream
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/end-of-stream
/usr/lib/node_modules/end-of-stream
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/es6-map /usr/lib/node_modules/es6-map
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/help-me /usr/lib/node_modules/help-me
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/inherits
/usr/lib/node_modules/inherits@2
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/minimist /usr/lib/node_modules/minimist
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/mqtt-packet
/usr/lib/node_modules/mqtt-packet
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/pump /usr/lib/node_modules/pump
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/readable-stream
/usr/lib/node_modules/readable-stream
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/reinterval
/usr/lib/node_modules/reinterval
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/split2 /usr/lib/node_modules/split2
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/websocket-stream
/usr/lib/node_modules/websocket-stream
  nodejs-mqtt.noarch: W: dangling-symlink
/usr/lib/node_modules/mqtt/node_modules/xtend /usr/lib/node_modules/xtend
  nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/test/helpers/public-cert.pem
  nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/test/helpers/tls-cert.pem
  nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/test/helpers/wrong-cert.pem
  nodejs-mqtt.noarch: W: no-manual-page-for-binary mqtt
  nodejs-mqtt.noarch: W: no-manual-page-for-binary mqtt_pub
  nodejs-mqtt.noarch: W: no-manual-page-for-binary mqtt_sub
  1 packages and 0 specfiles checked; 0 errors, 24 warnings.

My review on the result above is as followings.

1.1. nodejs-mqtt.src: W: no-%build-section
I think this warning is meaningless because this module is neither a native
module nor needs no build processes and has nothing to do in %build section.

1.2. nodejs-mqtt.noarch: W: incoherent-version-in-changelog 2.18.8-2
['2.18.8-1.fc29', '2.18.8-1']
The entry of 2.18.8-1 should exist because it must be the initial version of
this package.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs

'Release: 1%{?dist}' should be 'Release: 2%{?dist}' because the latest version
in the %changelog section is 2.18.8-2.

1.3. nodejs-mqtt.noarch: W: only-non-binary-in-usr-lib
You should ignore this because the following guideline says node.js modules
written purely in JavaScript should be installed to the %nodejs_sitelib.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_macros

1.4. nodejs-mqtt.noarch: W: pem-certificate
/usr/lib/node_modules/mqtt/examples...
I think this warning can be ignored because this PEM certificate will be used
for example purpose.

1.5. nodejs-mqtt.noarch: W: dangling-symlink ...
I think dangling-symlink warnings should be ignored the %nodejs_symlink_deps
macro creates a node_modules tree with symlinks for each dependency listed in
package.json.

1.6. nodejs-mqtt.noarch: W: pem-certificate /usr/lib/node_modules/mqtt/test...
I think this can be ignored because this PEM certificate will be used for test
purpose.

1.7. nodejs-mqtt.noarch: W: no-manual-page-for-binary ...
This should be fixed. You might know that help2man is a useful tool.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages

2. The 'Group' tag should not be used
-------------------------------------

The Group: tag should not be used. Here is the guideline.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_section...

3. Other suggestions
--------------------

You can use the automatic dependency generator in nodejs-packaging that adds
versioned dependencies based on the information provided in a module’s
package.json file.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_automati...

You can patch package.json if some dependency module's versions in it are 
incorrect by using the %nodejs_fixdep macro in nodejs-packaging.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_correcti...

The guideline says if a test suite should be executed if the package provides
it and it is practical to do so.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites

Appendix 1: Koji scratch build succeeded
----------------------------------------
https://koji.fedoraproject.org/koji/taskinfo?taskID=32688759

Thanks in advance,
Hirotaka Wakabayashi

[Bug 1667935] Review request nodejs-mqtt - MQTT client library for nodejs

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

Robert-André Mauchin  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Robert-André Mauchin  ---
 - You don't need to specify Requires for nodejs package
https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_automati...

needinfo canceled: [Bug 1667935] Review request nodejs-mqtt - MQTT client library for nodejs

Product: Fedora
Version: rawhide
Component: Package Review

Package Review  has canceled Package
Review 's request for Tomas Hrcka
's needinfo:
Bug 1667935: Review request nodejs-mqtt - MQTT client library for nodejs
https://bugzilla.redhat.com/show_bug.cgi?id=1667935

--- Comment #4 from Package Review  ---
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.