Re: [PATCH v4 09/14] remote-testgit: report success after an import
On 11/02/2012 03:02 AM, Felipe Contreras wrote:
Doesn't make a difference for the tests, but it does for the ones
seeking reference.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
git-remote-testgit | 11 +++
1 file changed, 11 insertions(+)
diff --git a/git-remote-testgit b/git-remote-testgit
index 6c348b0..4e8b356 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -59,7 +59,18 @@ while read line; do
sed -e s#refs/heads/#${prefix}/heads/#g
;;
export)
+declare -A before after
+
If you convert this script to be a valid POSIX shell script (as I've
suggested in my reply to [PATCH v4 04/14]), you'll need to get rid of
this bashism (and those below) as well.
+eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')
+
git fast-import --{import,export}-marks=$testgitmarks --quiet
+
+eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')
+
+for ref in ${!after[@]}; do
+test ${before[$ref]} == ${after[$ref]} continue
+echo ok $ref
+done
echo
;;
'')
Regards,
Stefano
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On Fri, Nov 2, 2012 at 2:58 PM, Stefano Lattarini
stefano.lattar...@gmail.com wrote:
On 11/02/2012 03:02 AM, Felipe Contreras wrote:
Doesn't make a difference for the tests, but it does for the ones
seeking reference.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
git-remote-testgit | 11 +++
1 file changed, 11 insertions(+)
diff --git a/git-remote-testgit b/git-remote-testgit
index 6c348b0..4e8b356 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -59,7 +59,18 @@ while read line; do
sed -e s#refs/heads/#${prefix}/heads/#g
;;
export)
+declare -A before after
+
If you convert this script to be a valid POSIX shell script (as I've
suggested in my reply to [PATCH v4 04/14]), you'll need to get rid of
this bashism (and those below) as well.
Yeah, but I don't want to. I originally used transitory files, and
used esoteric options of diff to do the same (which were probably not
portable).
In the end I liked this approach much better.
If you have a solution for this that works in POSIX shell, I'll be
glad to consider it, but for the moment, I think a simple, easy to
understand and maintain code is more important, and if it needs bash,
so be it.
+eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')
+
git fast-import --{import,export}-marks=$testgitmarks --quiet
+
+eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')
+
+for ref in ${!after[@]}; do
+test ${before[$ref]} == ${after[$ref]} continue
+echo ok $ref
+done
echo
;;
'')
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On 11/02/2012 04:46 PM, Felipe Contreras wrote: In the end I liked this approach much better. If you have a solution for this that works in POSIX shell, I'll be glad to consider it, but for the moment, I think a simple, easy to understand and maintain code is more important, and if it needs bash, so be it. If this is a deliberate decision, it's ok with me. I'm just a casual reviewer here, not an active contributor, so I'll gladly accept preferences and decisions of the active crew, once it's clear that they are deliberate and not the result of mistakes or confusion. In any case, I agree that having a clean, understandable code as a starting point is better than having a more portable but trickier one right away. If it will need converting to POSIX, that can be done as a follow up (and as we've both noticed, this would be the only point where such a conversion might be problematic -- the other changes would be trivial, almost automatic). Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On Fri, Nov 2, 2012 at 5:08 PM, Stefano Lattarini stefano.lattar...@gmail.com wrote: On 11/02/2012 04:46 PM, Felipe Contreras wrote: In the end I liked this approach much better. If you have a solution for this that works in POSIX shell, I'll be glad to consider it, but for the moment, I think a simple, easy to understand and maintain code is more important, and if it needs bash, so be it. If this is a deliberate decision, it's ok with me. I'm just a casual reviewer here, not an active contributor, so I'll gladly accept preferences and decisions of the active crew, once it's clear that they are deliberate and not the result of mistakes or confusion. In any case, I agree that having a clean, understandable code as a starting point is better than having a more portable but trickier one right away. If it will need converting to POSIX, that can be done as a follow up (and as we've both noticed, this would be the only point where such a conversion might be problematic -- the other changes would be trivial, almost automatic). As things are the options are: 1) Remove this code and move to POSIX sh. People looking for reference might scratch their heads as to why 'git push' is not showing the update. 2) Keep this code and remain in bash. Until we have a: 3) Replace this code with a clean POSIX sh alternative I would rather vote for 2) Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On 11/02/2012 05:19 PM, Felipe Contreras wrote: [SNIP] As things are the options are: 1) Remove this code and move to POSIX sh. People looking for reference might scratch their heads as to why 'git push' is not showing the update. 2) Keep this code and remain in bash. Until we have a: 3) Replace this code with a clean POSIX sh alternative I would rather vote for 2) That's perfectly fine with me, now that you've explained the rationale for requiring bash in the first place (maybe adding a comment to the script or the commit messages that reports this rationale for choosing to require bash might be worthwhile; but it's no big deal anyway). Thanks, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On Fri, Nov 02, 2012 at 05:19:33PM +0100, Felipe Contreras wrote: In any case, I agree that having a clean, understandable code as a starting point is better than having a more portable but trickier one right away. If it will need converting to POSIX, that can be done as a follow up (and as we've both noticed, this would be the only point where such a conversion might be problematic -- the other changes would be trivial, almost automatic). As things are the options are: 1) Remove this code and move to POSIX sh. People looking for reference might scratch their heads as to why 'git push' is not showing the update. 2) Keep this code and remain in bash. Until we have a: 3) Replace this code with a clean POSIX sh alternative I would rather vote for 2) I'm fine with bash. The critical thing is that it not break people's make test if they do not have bash (or do not have bash as /bin/bash). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/14] remote-testgit: report success after an import
Doesn't make a difference for the tests, but it does for the ones
seeking reference.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
git-remote-testgit | 11 +++
1 file changed, 11 insertions(+)
diff --git a/git-remote-testgit b/git-remote-testgit
index 6c348b0..4e8b356 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -59,7 +59,18 @@ while read line; do
sed -e s#refs/heads/#${prefix}/heads/#g
;;
export)
+declare -A before after
+
+eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')
+
git fast-import --{import,export}-marks=$testgitmarks --quiet
+
+eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')
+
+for ref in ${!after[@]}; do
+test ${before[$ref]} == ${after[$ref]} continue
+echo ok $ref
+done
echo
;;
'')
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
