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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Hirotaka Wakabayashi <hiwkby@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hiwkby@xxxxxxxxx



--- Comment #1 from Hirotaka Wakabayashi <hiwkby@xxxxxxxxx> ---
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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux