| Message ID | 1456335081-4834-1-git-send-email-universeII@gmx.de |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Andreas Ehmanns, On Wed, 24 Feb 2016 18:31:21 +0100, Andreas Ehmanns wrote: > Signed-off-by: Andreas Ehmanns <universeII@gmx.de> > --- > package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++--------------------- > 1 file changed, 56 insertions(+), 42 deletions(-) While your patch looks OK, it is doing *much* more than "adding OK/FAIL" as your commit title indicates. Ideally, you should separate your changes into multiple patches to make it easier to review the changes (like one patch to change to use functions, one patch to add OK/FAIL, one patch to remove set -e, etc.). Or at least, write a commit log that details all the changes that are being done. Thanks a lot! Thomas
Dear Thomas, thanks for the feedback. I will prepare several patches including smaller changes and re-submit them. Regards, Andreas Am 25.02.2016 um 10:46 schrieb Thomas Petazzoni: > Dear Andreas Ehmanns, > > On Wed, 24 Feb 2016 18:31:21 +0100, Andreas Ehmanns wrote: >> Signed-off-by: Andreas Ehmanns <universeII@gmx.de> >> --- >> package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++--------------------- >> 1 file changed, 56 insertions(+), 42 deletions(-) > While your patch looks OK, it is doing *much* more than "adding > OK/FAIL" as your commit title indicates. > > Ideally, you should separate your changes into multiple patches to make > it easier to review the changes (like one patch to change to use > functions, one patch to add OK/FAIL, one patch to remove set -e, etc.). > Or at least, write a commit log that details all the changes that are > being done. > > Thanks a lot! > > Thomas
Andreas, On Thu, 25 Feb 2016 21:02:07 +0100, Andreas Ehmanns wrote: > thanks for the feedback. I will prepare several patches including > smaller changes and re-submit them. Thanks a lot! Thomas
Thomas, one "stupid" questions. Shall the patches be incremental or relative to the original file? Example: Change some lines and create the first patch. Then change the next things based on the original file or based on the already patched file? Regards, Andreas Am 25.02.2016 um 21:25 schrieb Thomas Petazzoni: > Andreas, > > On Thu, 25 Feb 2016 21:02:07 +0100, Andreas Ehmanns wrote: > >> thanks for the feedback. I will prepare several patches including >> smaller changes and re-submit them. > Thanks a lot! > > Thomas
Hello, On Thu, 25 Feb 2016 21:43:48 +0100, Andreas Ehmanns wrote: > one "stupid" questions. Shall the patches be incremental or relative to > the original file? > Example: Change some lines and create the first patch. Then change the > next things based on the original file or based on the already patched file? Based on the already patched file of course, otherwise you won't go very far. You need to create a patch series. To do this, create a Git branch, make a set of commits that stack on top of each other, and then use git format-patch to create one patch file for each commit in your branch, and send them with git send-mail. You can look in the Buildroot manual, there are some explanations about this: http://nightly.buildroot.org/#submitting-patches Best regards, Thomas
Thomas, thanks for the clarification. Regards, Andreas Am 25.02.2016 um 22:04 schrieb Thomas Petazzoni: > Hello, > > On Thu, 25 Feb 2016 21:43:48 +0100, Andreas Ehmanns wrote: > >> one "stupid" questions. Shall the patches be incremental or relative to >> the original file? >> Example: Change some lines and create the first patch. Then change the >> next things based on the original file or based on the already patched file? > Based on the already patched file of course, otherwise you won't go > very far. You need to create a patch series. To do this, create a Git > branch, make a set of commits that stack on top of each other, and then > use git format-patch to create one patch file for each commit in your > branch, and send them with git send-mail. > > You can look in the Buildroot manual, there are some explanations about > this: > > http://nightly.buildroot.org/#submitting-patches > > Best regards, > > Thomas
diff --git a/package/netsnmp/S59snmpd b/package/netsnmp/S59snmpd index 4eea512..94773db 100755 --- a/package/netsnmp/S59snmpd +++ b/package/netsnmp/S59snmpd @@ -1,4 +1,4 @@ -#! /bin/sh -e +#! /bin/sh ### BEGIN INIT INFO # Provides: snmpd snmptrapd # Required-Start: $network $local_fs @@ -11,7 +11,6 @@ # # Author: Jochen Friedrich <jochen@scram.de> # -set -e export PATH=/sbin:/usr/sbin:/bin:/usr/bin @@ -38,56 +37,71 @@ if [ "$SNMPDCOMPAT" = "yes" ]; then ln -sf /var/agentx/master /var/run/agentx fi -case "$1" in - start) - printf "Starting network management services:" +start() { if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then - start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS - printf " snmpd" + printf "Starting SNMP daemon: " + start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS + [ $? = 0 ] && echo "OK" || echo "FAIL" fi + if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then - start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS - printf " snmptrapd" + printf "Starting SNMP trap daemon: " + start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS + [ $? = 0 ] && echo "OK" || echo "FAIL" fi - echo "." - ;; - stop) - printf "Stopping network management services:" - start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd - printf " snmpd" - start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd - printf " snmptrapd" - echo "." - ;; - restart) - printf "Restarting network management services:" - start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd - start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd - # Allow the daemons time to exit completely. - sleep 2 +} + +stop() { if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then - start-stop-daemon -q -S -x /usr/sbin/snmpd -- $SNMPDOPTS - printf " snmpd" + printf "Stopping SNMP daemon: " + start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmpd + [ $? = 0 ] && echo "OK" || echo "FAIL" fi + if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then - # Allow snmpd time to start up. - sleep 1 - start-stop-daemon -q -S -x /usr/sbin/snmptrapd -- $TRAPDOPTS - printf " snmptrapd" + printf "Stopping SNMP trap daemon: " + start-stop-daemon -q -K $ssd_oknodo -x /usr/sbin/snmptrapd + [ $? = 0 ] && echo "OK" || echo "FAIL" fi - echo "." - ;; - reload|force-reload) - printf "Reloading network management services:" +} + +reload() { if [ "$SNMPDRUN" = "yes" -a -f /etc/snmp/snmpd.conf ]; then - start-stop-daemon -q -K -s 1 -p /var/run/snmpd.pid -x /usr/sbin/snmpd - printf " snmpd" + printf "Reloading SNMP daemon: " + start-stop-daemon -q -K -s 1 -p /var/run/snmpd.pid -x /usr/sbin/snmpd + [ $? = 0 ] && echo "OK" || echo "FAIL" + fi + + if [ "$TRAPDRUN" = "yes" -a -f /etc/snmp/snmptrapd.conf ]; then + printf "Reloading SNMP trap daemon: " + start-stop-daemon -q -K -s 1 -p /var/run/snmptrapd.pid -x /usr/sbin/snmptrapd + [ $? = 0 ] && echo "OK" || echo "FAIL" fi - echo "." - ;; - *) - echo "Usage: /etc/init.d/snmpd {start|stop|restart|reload|force-reload}" - exit 1 +} + +case "$1" in + start) + start + ;; + + stop) + stop + ;; + + restart) + stop + # Allow the daemons time to exit completely. + sleep 2 + start + ;; + + reload|force-reload) + reload + ;; + + *) + echo "Usage: $0 {start|stop|restart|reload|force-reload}" + exit 1 esac exit 0
Signed-off-by: Andreas Ehmanns <universeII@gmx.de> --- package/netsnmp/S59snmpd | 98 +++++++++++++++++++++++++++--------------------- 1 file changed, 56 insertions(+), 42 deletions(-)