All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fixup-builtins: remove unused cruft
@ 2013-06-28 15:46 Ramkumar Ramachandra
  2013-06-28 16:45 ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-28 15:46 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

The fixup-builtins script is only used by an unused remove-dashes target
in the Makefile: remove that along with the script.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Makefile       |  3 ---
 fixup-builtins | 16 ----------------
 2 files changed, 19 deletions(-)
 delete mode 100755 fixup-builtins

diff --git a/Makefile b/Makefile
index e158376..6b0ddf8 100644
--- a/Makefile
+++ b/Makefile
@@ -2302,9 +2302,6 @@ check: common-cmds.h
 		exit 1; \
 	fi
 
-remove-dashes:
-	./fixup-builtins $(BUILT_INS) $(PROGRAMS) $(SCRIPTS)
-
 ### Installation rules
 
 ifneq ($(filter /%,$(firstword $(template_dir))),)
diff --git a/fixup-builtins b/fixup-builtins
deleted file mode 100755
index 63dfa4c..0000000
--- a/fixup-builtins
+++ /dev/null
@@ -1,16 +0,0 @@
-#!/bin/sh
-while [ "$1" ]
-do
-	if [ "$1" != "git-sh-setup" -a "$1" != "git-parse-remote" -a "$1" != "git-svn" ]; then
-		old="$1"
-		new=$(echo "$1" | sed 's/git-/git /')
-		echo "Converting '$old' to '$new'"
-		sed -i "s/\\<$old\\>/$new/g" $(git ls-files '*.sh')
-	fi
-	shift
-done
-
-sed -i 's/git merge-one-file/git-merge-one-file/g
-s/git rebase-todo/git-rebase-todo/g' $(git ls-files '*.sh')
-git update-index --refresh >& /dev/null
-exit 0
-- 
1.8.3.1.643.gebeea52.dirty

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] fixup-builtins: remove unused cruft
  2013-06-28 15:46 [PATCH] fixup-builtins: remove unused cruft Ramkumar Ramachandra
@ 2013-06-28 16:45 ` Jeff King
  2013-06-28 17:27   ` Ramkumar Ramachandra
  2013-06-28 18:49   ` Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: Jeff King @ 2013-06-28 16:45 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano

On Fri, Jun 28, 2013 at 09:16:19PM +0530, Ramkumar Ramachandra wrote:

> The fixup-builtins script is only used by an unused remove-dashes target
> in the Makefile: remove that along with the script.

I am not sure of this justification. If you read the commit message from
36e5e70, which introduced the target, it was meant to be run manually as
part of migrating away from dashed forms.

Running it today does still uncover some places that could be tweaked.
However, I note that it also has a lot of false positives (e.g.,
removing dashes from places that are not commands, like filenames or
config options). It also produces crappy output for documentation (if we
do want to change dashed mentions, we would probably want to switch
git-foo to `git foo` with some kind of quoting to make it more obvious).

So I think it is probably a good idea to remove it, but the
justification is not "this is unused cruft", but more like:

  This script was added in 36e5e70 (Start deprecating "git-command" in
  favor of "git command", 2007-06-30) with the intent of aiding the
  transition away from dashed forms. However, nobody is really working
  on that transition, and even if they did, this tool will probably lead
  them in the wrong direction, as it produces a large number of
  questionable changes.

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fixup-builtins: remove unused cruft
  2013-06-28 16:45 ` Jeff King
@ 2013-06-28 17:27   ` Ramkumar Ramachandra
  2013-06-28 18:49   ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-28 17:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Git List, Junio C Hamano

Jeff King wrote:
>   This script was added in 36e5e70 (Start deprecating "git-command" in
>   favor of "git command", 2007-06-30) with the intent of aiding the
>   transition away from dashed forms. However, nobody is really working
>   on that transition, and even if they did, this tool will probably lead
>   them in the wrong direction, as it produces a large number of
>   questionable changes.

Sure, that sounds good.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fixup-builtins: remove unused cruft
  2013-06-28 16:45 ` Jeff King
  2013-06-28 17:27   ` Ramkumar Ramachandra
@ 2013-06-28 18:49   ` Junio C Hamano
  2013-07-03 10:27     ` Ramkumar Ramachandra
  1 sibling, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2013-06-28 18:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Ramkumar Ramachandra, Git List

Jeff King <peff@peff.net> writes:

> So I think it is probably a good idea to remove it, but the
> justification is not "this is unused cruft", but more like:
>
>   This script was added in 36e5e70 (Start deprecating "git-command" in
>   favor of "git command", 2007-06-30) with the intent of aiding the
>   transition away from dashed forms. However, nobody is really working
>   on that transition, and even if they did, this tool will probably lead
>   them in the wrong direction, as it produces a large number of
>   questionable changes.

It is not "nobody is working on", but "it has already been used to
help the transision and served its purpose, and is no longer is very
useful for follow-up work because the majority of remaining matches
it finds are false positives."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fixup-builtins: remove unused cruft
  2013-06-28 18:49   ` Junio C Hamano
@ 2013-07-03 10:27     ` Ramkumar Ramachandra
  0 siblings, 0 replies; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-07-03 10:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Git List

Junio C Hamano wrote:
> It is not "nobody is working on", but "it has already been used to
> help the transision and served its purpose, and is no longer is very
> useful for follow-up work because the majority of remaining matches
> it finds are false positives."

Please amend the commit message appropriately (you clearly know best) and queue.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-07-03 10:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-28 15:46 [PATCH] fixup-builtins: remove unused cruft Ramkumar Ramachandra
2013-06-28 16:45 ` Jeff King
2013-06-28 17:27   ` Ramkumar Ramachandra
2013-06-28 18:49   ` Junio C Hamano
2013-07-03 10:27     ` Ramkumar Ramachandra

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.