Message ID | 1377047460-1937-1-git-send-email-danomimanchego123@gmail.com |
---|---|
State | Accepted |
Commit | 0be2fe9a8c07d1f58ca9eb288cb75aaae40550cd |
Headers | show |
Hi Danomi, Op 21-aug.-2013 03:11 schreef "Danomi Manchego" <danomimanchego123@gmail.com> het volgende: > > Several of the lines in S50sshd script have a strange mix of spaces > and tabs, that at least do not look consistent with neighboring lines. > This patch makes the spacing consistent, and also strips the trailing > spaces. > > Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com> > --- I haven't checked if there are such problems, but have you considered checking all init scripts for the same type of problems and fixing them in this same patch? A rough way of finding these scripts is: find -name "S*" Thanks, Thomas
On Tue, Aug 20, 2013 at 11:39 PM, Thomas De Schampheleire < patrickdepinguin@gmail.com> wrote: > I haven't checked if there are such problems, but have you considered > checking all init scripts for the same type of problems and fixing them in > this same patch? > Thomas, I looked through the other init scripts. There's some variation, mostly in: * Tabs vs spacing. * Where spaces are used instead of tabs, sometimes two spaces are used per level, other times four. * Indentation of start/stop under the case "$1" / esac clause. * Some of the start/stop begin in the first column, aligned with 'case'. * Some of them have two spaces, followed by actions offset with a tabs. * Some of them have two spaces, followed by actions offset with a sapces. * Some are offset with tab, followed by actions offset with two tabs. So I took the approach of leaving that part alone, and just fixing the lines that were offset by a mix of spaces and tabs, since that seemed clearly bad. I'd be willing to go through all of them and make them uniform, if recommended guidelines can be stated with regards to the above variations. Tabs are tempting only because it's a single character in text files that go in the target - otherwise, I'm normally in the tabs-are-evil camp, since thing which start out with tabs tend get modified with spaces over time, and because editor rendering of tab width can vary. So I'd be willing to go either way. Danomi -
Hi Danomi, On Wed, Aug 21, 2013 at 3:39 PM, Danomi Manchego <danomimanchego123@gmail.com> wrote: > On Tue, Aug 20, 2013 at 11:39 PM, Thomas De Schampheleire > <patrickdepinguin@gmail.com> wrote: >> >> I haven't checked if there are such problems, but have you considered >> checking all init scripts for the same type of problems and fixing them in >> this same patch? > > > Thomas, > > I looked through the other init scripts. There's some variation, mostly in: > > * Tabs vs spacing. > * Where spaces are used instead of tabs, sometimes two spaces are used per > level, other times four. > * Indentation of start/stop under the case "$1" / esac clause. > * Some of the start/stop begin in the first column, aligned with 'case'. > * Some of them have two spaces, followed by actions offset with a tabs. > * Some of them have two spaces, followed by actions offset with a > sapces. > * Some are offset with tab, followed by actions offset with two tabs. > > So I took the approach of leaving that part alone, and just fixing the lines > that were offset by a mix of spaces and tabs, since that seemed clearly bad. > > I'd be willing to go through all of them and make them uniform, if > recommended guidelines can be stated with regards to the above variations. > Tabs are tempting only because it's a single character in text files that go > in the target - otherwise, I'm normally in the tabs-are-evil camp, since > thing which start out with tabs tend get modified with spaces over time, and > because editor rendering of tab width can vary. So I'd be willing to go > either way. Thanks for checking. Seems there was more variance than I expected. I'm not sure what to do here. Unless such init scripts have been copied from elsewhere (in which making cosmetic changes may be undesirable), I'm ok with having them changed according to a given guideline. Before trying to establish such a guideline, what does the community think about this? If there is no consensus in changing, then no need for a guideline. Thanks, Thomas
On 22/08/13 15:59, Thomas De Schampheleire wrote: >> >I'd be willing to go through all of them and make them uniform, if >> >recommended guidelines can be stated with regards to the above variations. >> >Tabs are tempting only because it's a single character in text files that go >> >in the target - otherwise, I'm normally in the tabs-are-evil camp, since >> >thing which start out with tabs tend get modified with spaces over time, and >> >because editor rendering of tab width can vary. So I'd be willing to go >> >either way. > Thanks for checking. Seems there was more variance than I expected. > > I'm not sure what to do here. Unless such init scripts have been > copied from elsewhere (in which making cosmetic changes may be > undesirable), I'm ok with having them changed according to a given > guideline. If it is copied from elsewhere then it probably shouldn't be in buildroot, but just copied from elsewhere. > Before trying to establish such a guideline, what does the community > think about this? > If there is no consensus in changing, then no need for a guideline. I guess that's a judgement call for Peter to make. Note that we also have this kind of whitespace inconsistencies in e.g. support/scripts. Regards, Arnout
>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> writes:
Danomi> Several of the lines in S50sshd script have a strange mix of spaces
Danomi> and tabs, that at least do not look consistent with neighboring lines.
Danomi> This patch makes the spacing consistent, and also strips the trailing
Danomi> spaces.
Danomi> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
Committed, thanks.
diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd index 89f1b9a..b65b3c8 100644 --- a/package/openssh/S50sshd +++ b/package/openssh/S50sshd @@ -23,7 +23,7 @@ if [ ! -f /etc/ssh_host_dsa_key ] ; then echo Generating DSA Key... echo THIS CAN TAKE A MINUTE OR TWO DEPENDING ON YOUR PROCESSOR! echo - /usr/bin/ssh-keygen -t dsa -f /etc/ssh_host_dsa_key -C '' -N '' + /usr/bin/ssh-keygen -t dsa -f /etc/ssh_host_dsa_key -C '' -N '' fi # Check for the SSH2 ECDSA key @@ -33,35 +33,35 @@ if [ ! -f /etc/ssh_host_ecdsa_key ]; then echo /usr/bin/ssh-keygen -t ecdsa -f /etc/ssh_host_ecdsa_key -C '' -N '' fi - + umask 077 start() { - echo -n "Starting sshd: " + echo -n "Starting sshd: " /usr/sbin/sshd touch /var/lock/sshd echo "OK" -} +} stop() { echo -n "Stopping sshd: " - killall sshd + killall sshd rm -f /var/lock/sshd - echo "OK" + echo "OK" } restart() { stop start -} +} case "$1" in start) - start + start ;; stop) - stop + stop ;; restart|reload) - restart + restart ;; *) echo "Usage: $0 {start|stop|restart}"
Several of the lines in S50sshd script have a strange mix of spaces and tabs, that at least do not look consistent with neighboring lines. This patch makes the spacing consistent, and also strips the trailing spaces. Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com> --- package/openssh/S50sshd | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)