Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: tcallawa@redhat.com QAContact: fedora-package-review@redhat.com
Spec URL: http://www.auroralinux.org/people/spot/review/ser2net.spec SRPM URL: http://www.auroralinux.org/people/spot/review/ser2net-2.3-1.fc6.src.rpm Description: ser2net provides a way for a user to connect from a network connection to a serial port. It provides all the serial port setup, a configuration file to configure the ports, a control login for modifying port parameters, monitoring ports, and controlling ports.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
jima@beer.tclug.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |jima@beer.tclug.org OtherBugsDependingO|163776 |163778 nThis| |
------- Additional Comments From jima@beer.tclug.org 2006-10-02 11:24 EST ------- Hmm, sounds vaguely like conserver, but I'll give this a shot. :-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
jima@beer.tclug.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO Flag| |needinfo?(tcallawa@redhat.co | |m)
------- Additional Comments From jima@beer.tclug.org 2006-10-02 12:15 EST ------- Using my own review checklist: http://beer.tclug.org/fedora-extras/review-checklist-1.1.txt
1. `rpmlint ser2net-2.3-1.fc?.*.rpm` returns: W: ser2net service-default-enabled /etc/rc.d/init.d/ser2net
More on this below (#38).
SRPM and -debuginfo package have no rpmlint output.
2. Package appears to meet Package Naming Guidelines. 3. Spec is ser2net.spec, check. 4. Package appears to follow Packaging Guidelines. 5. Upstream site lists package as GPL. 6. Spec agrees. 7. %doc contains COPYING. 8. Spec appears to be American English. 9. Spec seems legible. 10. Tarball md5 matches upstream (5f83a3e8aec18331cb61069dccdfba47). 11. Package builds under FC5/i386, FC5/ppc, and devel/i386. 12. n/a, unless it fails under x86_64. 13. Package builds in Plague, so I imagine all necessary BRs are included. 14. Package does not appear to attempt to handle locales either properly nor improperly. 15. n/a, no library files. 16. Package does not appear to be designed to be relocatable. 17. Package owns all directories it creates. 18. No duplicate files. 19. Permissions appear to be sane. 20. Spec contains valid %clean section. 21. Macro use appears consistent. 22. Package contains code, not content. 23. %doc is minimal. 24. %doc doesn't affect runtime. 25. n/a, no header files or static libraries. 26. n/a, no .pc files. 27. n/a, no library files. 28. n/a, no -devel subpackage. 29. n/a, no .la files. 30. n/a, not a GUI application. 31. Package doesn't appear to have file conflicts with other packages. 32. Release tag contains %{?dist}. 33. n/a, already contains (and uses) COPYING. 34. n/a, no translations. 35. Package builds in Plague for FC5/i386, FC5/ppc, & devel/i386. 36. I can't verify x86_64, but package builds everywhere else, yes. 37. Package works on FC5/i386, at least. Neat package, too. 38. Scriptlet use appears to violate documented protocol:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-69c816fcf14e5...
From my understanding of this text, and other package reviews, services should
not be automatically enabled, especially without checking whether the transaction is an installation or an upgrade.
39. n/a, no subpackages.
Unless I'm mistaken (which, admittedly, is quite possible), I don't believe this package quite passes review. (Sorry...)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED Flag|needinfo?(tcallawa@redhat.co| |m) |
------- Additional Comments From tcallawa@redhat.com 2006-10-04 11:52 EST ------- Updated it to not turn the service on in the spec, but leave it enabled by default in the init script.
New SRPM: http://www.auroralinux.org/people/spot/review/ser2net-2.3-2.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ser2net.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
jima@beer.tclug.org changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778 |163779 nThis| |
------- Additional Comments From jima@beer.tclug.org 2006-10-04 14:11 EST ------- A slight typo on line 44 of your spec introduced this warning:
W: ser2net spurious-bracket-in-%preun
Putting a space between 0 and ] fixes that, leaving only:
W: ser2net service-default-enabled /etc/rc.d/init.d/ser2net
Assuming that's not a blocker (as my sponsor, I'm going to trust you on that, especially since I can't find anything saying otherwise), fix line 44 and I think we can call ser2net APPROVED.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
------- Additional Comments From ville.skytta@iki.fi 2006-10-04 14:18 EST ------- If the service is enabled by default in the init script, chkconfig --add will make it enabled by default. Is there a good reason for doing so?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
------- Additional Comments From tcallawa@redhat.com 2006-10-04 15:48 EST ------- Yes, but will chkconfig --add enable it on an upgrade if it has been disabled by the end user?
Upstream enables this by default in their init script, and I tend to fall into the camp of "if you didn't want it enabled, you wouldn't have installed it".
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
------- Additional Comments From tcallawa@redhat.com 2006-10-18 16:56 EST ------- New packages which fix the typo:
New SRPM: http://www.auroralinux.org/people/spot/review/ser2net-2.3-3.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ser2net.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=208680
tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
------- Additional Comments From tcallawa@redhat.com 2006-10-18 17:49 EST ------- Committed, built, and done for now.
Thanks for the review.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ser2net - Proxy that allows tcp connections to serial ports
https://bugzilla.redhat.com/show_bug.cgi?id=208680
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide
package-review@lists.fedoraproject.org