[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH] for asvn

From: Scott Miller <Scott.Miller_at_prioritytech.com>
Date: 2006-10-17 16:35:32 CEST

Peter,

Thanks for the reply.

I apologize for having outlook not attach it as a text MIME type. I'd
spent quite a while ensuring the transfer back and forth between unix
and winxp didn't munge the cr/lf encoding, so I'd glossed over the MIME
type of the attachment when I finally sent the email.

Indentation: I'm simply using vim with a tab width set at 4, it's simply
a standard that I've gotten used to over the years, and is certainly
easily changed to something else depending on others' preferences.
Thanks for pointing out the area I'd messed up, I will fix it. Really
the indentation changes were made while I was first looking over the
code to keep myself from getting confused. I mainly work with Perl now,
shell scripting is a distant second, and I was a C guy before that, so
I'm always baffled at some of the indentation "conventions" that are
followed with shell scripting; they don't seem to be followed very well
or consistently, or it could be that I simply don't understand the
rules.

And finally, the "interesting location" for subversion is due to having
read the svn manual portion that warns about ensuring the umask is set
to something sane like 002, so the "normal location" for subversion is a
set of shell scripts that set the umask then call the actual binaries.
For some as yet unexplored reason, the extra shell redirection kept the
svn calls from working; so I'd "fixed" that by calling the binary
directly. I'd forgotten about that little detail before sending the
patch. That change should be removed (and I need to figure out why that
was failing).

Are there any substantive type changes that should be made before I
submit a new patch?

-Scott

-----Original Message-----
From: Peter Samuelson [mailto:peter@p12n.org]
Sent: Monday, October 16, 2006 7:03 PM
To: Scott Miller
Cc: dev@subversion.tigris.org
Subject: Re: [PATCH] for asvn

[Scott Miller]
> o Changed indentation to follow a consistent style.

A couple things. (I'm not involved with asvn in any way, mind you.)
First, your patch got attached as a binary file (MIME type
application/octet-stream) so most mail readers, including mine, didn't
just show it as text - I had to jump through a small hoop to read it.
Try to convince your mail reader to give it a "text/something" type
next time, then it's more convenient to review.

Second, regarding your indentation changes:

@@ -62,9 +82,9 @@
     ref=`expr "$dir" : "$refname/\(.*\)"`
     if [ -z "$ref" ]
     then
- echo .
- else
- echo $ref
+ echo .
+ else
+ echo $ref
     fi

That looks quite misguided to me. First, you messed up the 'else'
line, which was correctly indented at 4 spaces.

Second, this script is for use on Unix, right? The default tab width
on Unix is 8 spaces, so the existing indentation was 4 spaces per
level. You changed it to 2 tabs, which is 16 spaces. (I've converted
tabs to spaces in this mail so you can see what it looks like.)

Your changes seem to indicate that you have a Unix system where tabs
are 4 spaces wide. I'm curious about what system that is.

Honestly, though, if one is going to use a basic indentation level of
anything other than 8, using tabs in the indents is IMO not a great
idea anyway - just use the appropriate number of spaces.

Finally:

@@ -382,7 +418,8 @@
 
 [ "$ACTION" = "pre" ] && pre_checkin $@
 
-$SVN $@
+umask 002
+/usr/local/subversion/bin/svn "$@"
 
 [ $? = 0 -a "$ACTION" = "post" ] && post_checkout $@

What is this /usr/local/subversion/bin/svn? Why did you not want to
use $SVN like everywhere else?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 17 16:37:13 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.