All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Luberda <robert@debian.org>
To: Eric Wong <normalperson@yhbt.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH/RFC] git svn: handle errors and concurrent commits in dcommit
Date: Wed, 08 Aug 2012 07:32:04 +0200	[thread overview]
Message-ID: <5021F9D4.1010700@debian.org> (raw)
In-Reply-To: <20120802104421.GA13271@dcvr.yhbt.net>

Eric Wong wrote:

Hi,

> 
> A few minor comments inline...
> Please ensure all error messages and code are readable in
> 80-column terminals.
> Also, keep opening "{" on the same line as the if/unless.
> Backticks don't nest properly, nowadays, we prefer:
> 	N=$(expr $N + 1)
>> +		cp auto_updated_file auto_updated_file_saved
> Need "&&" to check for failure on cp
>> +		sed -i 1d auto_updated_file && git commit -am "commit change $N.3" &&
> I don't believe "sed -i" is portable enough for this test.


Many thanks for the comments! I've fixed all of the above and will send
updated patch in next e-mail. Please let me know if you have any further
comments.



>> +		echo "PATH=\"$PATH\"; export PATH" >> $hook
>> +		echo "svnconf=\"$svnconf\"" >> $hook
>> +		cat >> "$hook" <<- 'EOF2'
>> +			cd work-auto-commits.svn
>> +			svn up --config-dir "$svnconf"
> 
> That doesn't seem to interact well with users who depend on
> svn_cmd pointing to something non-standard.  Not sure
> what to do about it, though....

I have no idea how to change it either. I've tried to source the
lib-git-svn.sh file inside the hook, but it sources test-lib.sh, and the
latter script doesn't work well if it is sourced by non-test script.
Anyway I the part of my original patch unchanged.

Regards,
robert

  reply	other threads:[~2012-08-08  5:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-01 21:26 [PATCH/RFC] git svn: handle errors and concurrent commits in dcommit Robert Luberda
2012-08-02 10:44 ` Eric Wong
2012-08-08  5:32   ` Robert Luberda [this message]
2012-08-08  5:35     ` Robert Luberda
2012-08-08 23:07     ` Eric Wong
2012-08-09 17:44       ` Junio C Hamano
2012-08-10 19:51         ` Eric Wong
2012-08-19 22:43           ` Robert Luberda
2012-08-20  1:20             ` Junio C Hamano
2012-08-21 22:01             ` Eric Wong
2012-08-24 23:47               ` Robert Luberda
2012-08-26  0:34                 ` Eric Wong
2012-08-31  6:29                   ` [PATCH] t9164: More style fixes Robert Luberda
2012-08-31 18:01                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5021F9D4.1010700@debian.org \
    --to=robert@debian.org \
    --cc=git@vger.kernel.org \
    --cc=normalperson@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.