Bug 1667935 - Review request nodejs-mqtt - MQTT client library for nodejs
Summary: Review request nodejs-mqtt - MQTT client library for nodejs
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2019-01-21 13:33 UTC by Tomas Hrcka
Modified: 2020-08-10 00:58 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-08-10 00:58:44 UTC
Type: Bug


Attachments (Terms of Use)

Description Tomas Hrcka 2019-01-21 13:33:47 UTC
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

Comment 1 Hirotaka Wakabayashi 2019-02-11 05:31:14 UTC
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_sections
   
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/#_automatic_requires_and_provides

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/#_correcting_dependencies

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

Comment 2 Robert-André Mauchin 🐧 2019-02-15 23:44:02 UTC
 - You don't need to specify Requires for nodejs package https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_automatic_requires_and_provides

Comment 3 Package Review 2020-07-10 00:57:05 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 4 Package Review 2020-08-10 00:58:44 UTC
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.


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