{"thread":{"id":"36348","subject":"Patch Series v3 for \"use the $( ... ) construct for command substitution\"","startedAt":"2014-04-04T16:52:03Z","lastAt":"2014-04-04T19:27:48Z","messageCount":5,"participants":["Elia Pinto","Jonathan Nieder","Matthieu Moy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"238373","messageId":"CA+EOSBkF+hLOab0oJH7HuUb_KKn+1sgV_Lshwupgj-_yAAfmFw@mail.gmail.com","threadId":"36348","inReplyTo":null,"subject":"Patch Series v3 for \"use the $( ... ) construct for command substitution\"","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2014-04-04T16:52:03Z","receivedAt":"2014-04-04T16:52:03Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"This patch series contain the\n\n use the $( ... ) construct for command substitution\n\npatches not already merged in ep/shell-command-substitution\nin the mantainer repository. It is the version 3 of the\npatch series.\n\nI changed the commit message in accordance with those approved,\nand I have rebased the patches to the master branch, adjusting where\nnecessary any conflicts. The patches can be applied also to pu and next.\n\nI hope this is a better way to send the (a long) patch series to git.\n\nBest Regards\n\n------\n\nThe following changes since commit 82edd396632501b3dd1901d0f3ae5aa72759b5fb:\n\n  Update draft release notes to 2.0 (2014-04-03 13:40:59 -0700)\n\nare available in the git repository at:\n\n  git@github.com:devzero2000/git-core.git ep/shell-command-substitution-v3\n\nfor you to fetch changes up to a4f27b5de27b9848c0720ac2c74a3b82c2531122:\n\n  t7505-prepare-commit-msg-hook.sh: use the $( ... ) construct for\ncommand substitution (2014-04-04 08:03:33 -0700)\n\n----------------------------------------------------------------\nElia Pinto (140):\n      howto-index.sh: use the $( ... ) construct for command substitution\n      install-webdoc.sh: use the $( ... ) construct for command substitution\n      git-checkout.sh: use the $( ... ) construct for command substitution\n      git-clone.sh: use the $( ... ) construct for command substitution\n      git-commit.sh: use the $( ... ) construct for command substitution\n      git-fetch.sh: use the $( ... ) construct for command substitution\n      git-ls-remote.sh: use the $( ... ) construct for command substitution\n      git-merge.sh: use the $( ... ) construct for command substitution\n      git-repack.sh: use the $( ... ) construct for command substitution\n      git-resolve.sh: use the $( ... ) construct for command substitution\n      git-revert.sh: use the $( ... ) construct for command substitution\n      git-tag.sh: use the $( ... ) construct for command substitution\n      t9360-mw-to-git-clone.sh: use the $( ... ) construct for command\nsubstitution\n      t9362-mw-to-git-utf8.sh: use the $( ... ) construct for command\nsubstitution\n      t9365-continuing-queries.sh: use the $( ... ) construct for\ncommand substitution\n      test-gitmw-lib.sh: use the $( ... ) construct for command substitution\n      t7900-subtree.sh: use the $( ... ) construct for command substitution\n      appp.sh: use the $( ... ) construct for command substitution\n      txt-to-pot.sh: use the $( ... ) construct for command substitution\n      git-pull.sh: use the $( ... ) construct for command substitution\n      git-rebase--merge.sh: use the $( ... ) construct for command substitution\n      git-rebase.sh: use the $( ... ) construct for command substitution\n      git-stash.sh: use the $( ... ) construct for command substitution\n      git-web--browse.sh: use the $( ... ) construct for command substitution\n      lib-credential.sh: use the $( ... ) construct for command substitution\n      lib-cvs.sh: use the $( ... ) construct for command substitution\n      lib-gpg.sh: use the $( ... ) construct for command substitution\n      p5302-pack-index.sh: use the $( ... ) construct for command substitution\n      t0001-init.sh: use the $( ... ) construct for command substitution\n      t0010-racy-git.sh: use the $( ... ) construct for command substitution\n      t0020-crlf.sh: use the $( ... ) construct for command substitution\n      t0025-crlf-auto.sh: use the $( ... ) construct for command substitution\n      t0026-eol-config.sh: use the $( ... ) construct for command substitution\n      t0030-stripspace.sh: use the $( ... ) construct for command substitution\n      t0300-credentials.sh: use the $( ... ) construct for command substitution\n      t1000-read-tree-m-3way.sh: use the $( ... ) construct for\ncommand substitution\n      t1001-read-tree-m-2way.sh: use the $( ... ) construct for\ncommand substitution\n      t1002-read-tree-m-u-2way.sh: use the $( ... ) construct for\ncommand substitution\n      t1003-read-tree-prefix.sh: use the $( ... ) construct for\ncommand substitution\n      t1004-read-tree-m-u-wf.sh: use the $( ... ) construct for\ncommand substitution\n      t1020-subdirectory.sh: use the $( ... ) construct for command substitution\n      t1050-large.sh: use the $( ... ) construct for command substitution\n      t1100-commit-tree-options.sh: use the $( ... ) construct for\ncommand substitution\n      t1401-symbolic-ref.sh: use the $( ... ) construct for command substitution\n      t1410-reflog.sh: use the $( ... ) construct for command substitution\n      t1511-rev-parse-caret.sh: use the $( ... ) construct for command\nsubstitution\n      t1512-rev-parse-disambiguation.sh: use the $( ... ) construct\nfor command substitution\n      t2102-update-index-symlinks.sh: use the $( ... ) construct for\ncommand substitution\n      t3030-merge-recursive.sh: use the $( ... ) construct for command\nsubstitution\n      t3100-ls-tree-restrict.sh: use the $( ... ) construct for\ncommand substitution\n      t3101-ls-tree-dirname.sh: use the $( ... ) construct for command\nsubstitution\n      t3210-pack-refs.sh: use the $( ... ) construct for command substitution\n      t3403-rebase-skip.sh: use the $( ... ) construct for command substitution\n      t3511-cherry-pick-x.sh: use the $( ... ) construct for command\nsubstitution\n      t3600-rm.sh: use the $( ... ) construct for command substitution\n      t3700-add.sh: use the $( ... ) construct for command substitution\n      t3905-stash-include-untracked.sh: use the $( ... ) construct for\ncommand substitution\n      t3910-mac-os-precompose.sh: use the $( ... ) construct for\ncommand substitution\n      t4006-diff-mode.sh: use the $( ... ) construct for command substitution\n      t4010-diff-pathspec.sh: use the $( ... ) construct for command\nsubstitution\n      t4012-diff-binary.sh: use the $( ... ) construct for command substitution\n      t4013-diff-various.sh: use the $( ... ) construct for command substitution\n      t4014-format-patch.sh: use the $( ... ) construct for command substitution\n      t4036-format-patch-signer-mime.sh: use the $( ... ) construct\nfor command substitution\n      t4038-diff-combined.sh: use the $( ... ) construct for command\nsubstitution\n      t4057-diff-combined-paths.sh: use the $( ... ) construct for\ncommand substitution\n      t4116-apply-reverse.sh: use the $( ... ) construct for command\nsubstitution\n      t4119-apply-config.sh: use the $( ... ) construct for command substitution\n      t4204-patch-id.sh: use the $( ... ) construct for command substitution\n      t5000-tar-tree.sh: use the $( ... ) construct for command substitution\n      t5003-archive-zip.sh: use the $( ... ) construct for command substitution\n      t5100-mailinfo.sh: use the $( ... ) construct for command substitution\n      t5300-pack-object.sh: use the $( ... ) construct for command substitution\n      t5301-sliding-window.sh: use the $( ... ) construct for command\nsubstitution\n      t5302-pack-index.sh: use the $( ... ) construct for command substitution\n      t5303-pack-corruption-resilience.sh: use the $( ... ) construct\nfor command substitution\n      t5304-prune.sh: use the $( ... ) construct for command substitution\n      t5305-include-tag.sh: use the $( ... ) construct for command substitution\n      t5500-fetch-pack.sh: use the $( ... ) construct for command substitution\n      t5505-remote.sh: use the $( ... ) construct for command substitution\n      t5506-remote-groups.sh: use the $( ... ) construct for command\nsubstitution\n      t5510-fetch.sh: use the $( ... ) construct for command substitution\n      t5515-fetch-merge-logic.sh: use the $( ... ) construct for\ncommand substitution\n      t5516-fetch-push.sh: use the $( ... ) construct for command substitution\n      t5517-push-mirror.sh: use the $( ... ) construct for command substitution\n      t5520-pull.sh: use the $( ... ) construct for command substitution\n      t5522-pull-symlink.sh: use the $( ... ) construct for command substitution\n      t5530-upload-pack-error.sh: use the $( ... ) construct for\ncommand substitution\n      t5537-fetch-shallow.sh: use the $( ... ) construct for command\nsubstitution\n      t5538-push-shallow.sh: use the $( ... ) construct for command substitution\n      t5550-http-fetch-dumb.sh: use the $( ... ) construct for command\nsubstitution\n      t5551-http-fetch-smart.sh: use the $( ... ) construct for\ncommand substitution\n      t5570-git-daemon.sh: use the $( ... ) construct for command substitution\n      t5601-clone.sh: use the $( ... ) construct for command substitution\n      t5700-clone-reference.sh: use the $( ... ) construct for command\nsubstitution\n      t5710-info-alternate.sh: use the $( ... ) construct for command\nsubstitution\n      t5900-repo-selection.sh: use the $( ... ) construct for command\nsubstitution\n      t6001-rev-list-graft.sh: use the $( ... ) construct for command\nsubstitution\n      t6002-rev-list-bisect.sh: use the $( ... ) construct for command\nsubstitution\n      t6015-rev-list-show-all-parents.sh: use the $( ... ) construct\nfor command substitution\n      t6032-merge-large-rename.sh: use the $( ... ) construct for\ncommand substitution\n      t6034-merge-rename-nocruft.sh: use the $( ... ) construct for\ncommand substitution\n      t6132-pathspec-exclude.sh: use the $( ... ) construct for\ncommand substitution\n      t7001-mv.sh: use the $( ... ) construct for command substitution\n      t7003-filter-branch.sh: use the $( ... ) construct for command\nsubstitution\n      t7004-tag.sh: use the $( ... ) construct for command substitution\n      t7006-pager.sh: use the $( ... ) construct for command substitution\n      t7103-reset-bare.sh: use the $( ... ) construct for command substitution\n      t7406-submodule-update.sh: use the $( ... ) construct for\ncommand substitution\n      t7408-submodule-reference.sh: use the $( ... ) construct for\ncommand substitution\n      t7504-commit-msg-hook.sh: use the $( ... ) construct for command\nsubstitution\n      t7602-merge-octopus-many.sh: use the $( ... ) construct for\ncommand substitution\n      t7700-repack.sh: use the $( ... ) construct for command substitution\n      t8003-blame-corner-cases.sh: use the $( ... ) construct for\ncommand substitution\n      t9001-send-email.sh: use the $( ... ) construct for command substitution\n      t9100-git-svn-basic.sh: use the $( ... ) construct for command\nsubstitution\n      t9101-git-svn-props.sh: use the $( ... ) construct for command\nsubstitution\n      t9104-git-svn-follow-parent.sh: use the $( ... ) construct for\ncommand substitution\n      t9105-git-svn-commit-diff.sh: use the $( ... ) construct for\ncommand substitution\n      t9107-git-svn-migrate.sh: use the $( ... ) construct for command\nsubstitution\n      t9108-git-svn-glob.sh: use the $( ... ) construct for command substitution\n      t9109-git-svn-multi-glob.sh: use the $( ... ) construct for\ncommand substitution\n      t9110-git-svn-use-svm-props.sh: use the $( ... ) construct for\ncommand substitution\n      t9114-git-svn-dcommit-merge.sh: use the $( ... ) construct for\ncommand substitution\n      t9118-git-svn-funky-branch-names.sh: use the $( ... ) construct\nfor command substitution\n      t9119-git-svn-info.sh: use the $( ... ) construct for command substitution\n      t9129-git-svn-i18n-commitencoding.sh: use the $( ... ) construct\nfor command substitution\n      t9130-git-svn-authors-file.sh: use the $( ... ) construct for\ncommand substitution\n      t9132-git-svn-broken-symlink.sh: use the $( ... ) construct for\ncommand substitution\n      t9137-git-svn-dcommit-clobber-series.sh: use the $( ... )\nconstruct for command substitution\n      t9138-git-svn-authors-prog.sh: use the $( ... ) construct for\ncommand substitution\n      t9145-git-svn-master-branch.sh: use the $( ... ) construct for\ncommand substitution\n      t9150-svk-mergetickets.sh: use the $( ... ) construct for\ncommand substitution\n      t9300-fast-import.sh: use the $( ... ) construct for command substitution\n      t9350-fast-export.sh: use the $( ... ) construct for command substitution\n      t9501-gitweb-standalone-http-status.sh: use the $( ... )\nconstruct for command substitution\n      t9901-git-web--browse.sh: use the $( ... ) construct for command\nsubstitution\n      test-lib-functions.sh: use the $( ... ) construct for command substitution\n      unimplemented.sh: use the $( ... ) construct for command substitution\n      t7505-prepare-commit-msg-hook.sh: use the $( ... ) construct for\ncommand substitution\n\n Documentation/howto-index.sh                    |   12 ++--\n Documentation/install-webdoc.sh                 |    6 +-\n contrib/examples/git-checkout.sh                |    8 +--\n contrib/examples/git-clone.sh                   |   20 +++----\n contrib/examples/git-commit.sh                  |   10 ++--\n contrib/examples/git-fetch.sh                   |    6 +-\n contrib/examples/git-ls-remote.sh               |    4 +-\n contrib/examples/git-merge.sh                   |    4 +-\ncontrib/examples/git-repack.sh                  |    2 +-\n contrib/examples/git-resolve.sh                 |    2 +-\n contrib/examples/git-revert.sh                  |    2 +-\n contrib/examples/git-tag.sh                     |    2 +-\n contrib/mw-to-git/t/t9360-mw-to-git-clone.sh    |   14 ++---\n contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh     |    4 +-\n contrib/mw-to-git/t/t9365-continuing-queries.sh |    2 +-\n contrib/mw-to-git/t/test-gitmw-lib.sh           |    6 +-\n contrib/subtree/t/t7900-subtree.sh              |    2 +-\n contrib/thunderbird-patch-inline/appp.sh        |   14 ++---\n git-gui/po/glossary/txt-to-pot.sh               |    4 +-\n git-pull.sh                                     |    2 +-\n git-rebase--merge.sh                            |    4 +-\n git-rebase.sh                                   |    8 +--\n git-stash.sh                                    |    2 +-\n git-web--browse.sh                              |    6 +-\n t/lib-credential.sh                             |    2 +-\n t/lib-cvs.sh                                    |    2 +-\n t/lib-gpg.sh                                    |    2 +-\n t/perf/p5302-pack-index.sh                      |    2 +-\n t/t0001-init.sh                                 |   12 ++--\n t/t0010-racy-git.sh                             |    4 +-\n t/t0020-crlf.sh                                 |   42 +++++++-------\n t/t0025-crlf-auto.sh                            |   38 ++++++-------\n t/t0026-eol-config.sh                           |   20 +++----\n t/t0030-stripspace.sh                           |   20 +++----\n t/t0300-credentials.sh                          |    2 +-\n t/t1000-read-tree-m-3way.sh                     |    4 +-\n t/t1001-read-tree-m-2way.sh                     |   18 +++---\n t/t1002-read-tree-m-u-2way.sh                   |   10 ++--\n t/t1003-read-tree-prefix.sh                     |    2 +-\n t/t1004-read-tree-m-u-wf.sh                     |    8 +--\n t/t1020-subdirectory.sh                         |   22 ++++----\n t/t1050-large.sh                                |    4 +-\n t/t1100-commit-tree-options.sh                  |    4 +-\n t/t1401-symbolic-ref.sh                         |    2 +-\n t/t1410-reflog.sh                               |   24 ++++----\n t/t1511-rev-parse-caret.sh                      |    4 +-\n t/t1512-rev-parse-disambiguation.sh             |    8 +--\n t/t2102-update-index-symlinks.sh                |    2 +-\n t/t3030-merge-recursive.sh                      |    2 +-\n t/t3100-ls-tree-restrict.sh                     |    2 +-\n t/t3101-ls-tree-dirname.sh                      |    2 +-\n t/t3210-pack-refs.sh                            |    2 +-\n t/t3403-rebase-skip.sh                          |    2 +-\n t/t3511-cherry-pick-x.sh                        |   14 ++---\n t/t3600-rm.sh                                   |    4 +-\n t/t3700-add.sh                                  |   16 +++---\n t/t3905-stash-include-untracked.sh              |    4 +-\n t/t3910-mac-os-precompose.sh                    |   16 +++---\n t/t4006-diff-mode.sh                            |    2 +-\n t/t4010-diff-pathspec.sh                        |    4 +-\n t/t4012-diff-binary.sh                          |   16 +++---\n t/t4013-diff-various.sh                         |    6 +-\n t/t4014-format-patch.sh                         |   10 ++--\n t/t4036-format-patch-signer-mime.sh             |    2 +-\n t/t4038-diff-combined.sh                        |    2 +-\n t/t4057-diff-combined-paths.sh                  |    2 +-\n t/t4116-apply-reverse.sh                        |   12 ++--\n t/t4119-apply-config.sh                         |    2 +-\nt/t4204-patch-id.sh                             |    4 +-\n t/t5000-tar-tree.sh                             |    6 +-\n t/t5003-archive-zip.sh                          |    2 +-\n t/t5100-mailinfo.sh                             |   12 ++--\n t/t5300-pack-object.sh                          |   18 +++---\n t/t5301-sliding-window.sh                       |   14 ++---\n t/t5302-pack-index.sh                           |   34 ++++++------\n t/t5303-pack-corruption-resilience.sh           |    8 +--\n t/t5304-prune.sh                                |    2 +-\n t/t5305-include-tag.sh                          |    8 +--\n t/t5500-fetch-pack.sh                           |   16 +++---\n t/t5505-remote.sh                               |    2 +-\n t/t5506-remote-groups.sh                        |    2 +-\n t/t5510-fetch.sh                                |   10 ++--\n t/t5515-fetch-merge-logic.sh                    |    4 +-\n t/t5516-fetch-push.sh                           |    4 +-\n t/t5517-push-mirror.sh                          |    2 +-\n t/t5520-pull.sh                                 |   10 ++--\n t/t5522-pull-symlink.sh                         |    2 +-\n t/t5530-upload-pack-error.sh                    |    2 +-\n t/t5537-fetch-shallow.sh                        |    4 +-\n t/t5538-push-shallow.sh                         |    4 +-\n t/t5550-http-fetch-dumb.sh                      |    8 +--\n t/t5551-http-fetch-smart.sh                     |    2 +-\n t/t5570-git-daemon.sh                           |    8 +--\n t/t5601-clone.sh                                |    2 +-\n t/t5700-clone-reference.sh                      |    2 +-\n t/t5710-info-alternate.sh                       |    2 +-\n t/t5900-repo-selection.sh                       |    2 +-\n t/t6001-rev-list-graft.sh                       |   12 ++--\n t/t6002-rev-list-bisect.sh                      |    6 +-\n t/t6015-rev-list-show-all-parents.sh            |    6 +-\n t/t6032-merge-large-rename.sh                   |    2 +-\n t/t6034-merge-rename-nocruft.sh                 |    2 +-\n t/t6132-pathspec-exclude.sh                     |    2 +-\n t/t7001-mv.sh                                   |    4 +-\n t/t7003-filter-branch.sh                        |    6 +-\n t/t7004-tag.sh                                  |   16 +++---\n t/t7006-pager.sh                                |    2 +-\n t/t7103-reset-bare.sh                           |    2 +-\n t/t7406-submodule-update.sh                     |    4 +-\n t/t7408-submodule-reference.sh                  |    2 +-\n t/t7504-commit-msg-hook.sh                      |    2 +-\n t/t7505-prepare-commit-msg-hook.sh              |   32 +++++------\n t/t7602-merge-octopus-many.sh                   |    8 +--\n t/t7700-repack.sh                               |    4 +-\n t/t8003-blame-corner-cases.sh                   |    4 +-\n t/t9001-send-email.sh                           |   10 ++--\n t/t9100-git-svn-basic.sh                        |   24 ++++----\n t/t9101-git-svn-props.sh                        |   30 +++++-----\n t/t9104-git-svn-follow-parent.sh                |   48 ++++++++--------\n t/t9105-git-svn-commit-diff.sh                  |    4 +-\n t/t9107-git-svn-migrate.sh                      |   16 +++---\n t/t9108-git-svn-glob.sh                         |   20 +++----\n t/t9109-git-svn-multi-glob.sh                   |   32 +++++------\n t/t9110-git-svn-use-svm-props.sh                |    2 +-\n t/t9114-git-svn-dcommit-merge.sh                |   12 ++--\n t/t9118-git-svn-funky-branch-names.sh           |    2 +-\n t/t9119-git-svn-info.sh                         |    2 +-\n t/t9129-git-svn-i18n-commitencoding.sh          |    4 +-\n t/t9130-git-svn-authors-file.sh                 |   12 ++--\n t/t9132-git-svn-broken-symlink.sh               |    4 +-\n t/t9137-git-svn-dcommit-clobber-series.sh       |   24 ++++----\n t/t9138-git-svn-authors-prog.sh                 |    2 +-\n t/t9145-git-svn-master-branch.sh                |    4 +-\n t/t9150-svk-mergetickets.sh                     |    2 +-\n t/t9300-fast-import.sh                          |   68 +++++++++++------------\n t/t9350-fast-export.sh                          |    6 +-\n t/t9501-gitweb-standalone-http-status.sh        |    6 +-\n t/t9901-git-web--browse.sh                      |    2 +-\n t/test-lib-functions.sh                         |    8 +--\n unimplemented.sh                                |    2 +-\n 140 files changed, 592 insertions(+), 592 deletions(-)\n"},{"id":"238374","messageId":"20140404172946.GI6851@google.com","threadId":"36348","inReplyTo":"CA+EOSBkF+hLOab0oJH7HuUb_KKn+1sgV_Lshwupgj-_yAAfmFw@mail.gmail.com","subject":"Re: Patch Series v3 for \"use the $( ... ) construct for command substitution\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-04T17:29:46Z","receivedAt":"2014-04-04T17:29:46Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nElia Pinto wrote:\n\n> This patch series contain the\n>\n>  use the $( ... ) construct for command substitution\n>\n> patches not already merged in ep/shell-command-substitution\n> in the mantainer repository.\n\nThanks for working on this.  The $() form is less error-prone\nthan ``, so in that sense it can be worthwhile.\n\n[...]\n> Elia Pinto (140):\n\nI admit I'm not excited to review these at all.\n\nI wonder if it's possible to make the series easier to review.  For\nexample:\n\n * patch 0 makes preparatory changes to line wrapping or to avoid\n   using `` some places to make an automatic transformation easier\n * patch 1 introduces a script to transform `` expressions to $()\n   expressions\n * patch 2 just runs that script\n\nIf the script is \"obviously correct\" enough then there is no need\nto manually go through 140 files after that point.\n\nIf the only way to get this done is to actually manually review those\n140 files, I just don't think it's worth it.  The `` construct is not\n*that* bad.  Another possible direction could be to add a tool to make\nsure git doesn't get any new uses of ``, to let the changes flow in at\na manageable rate without too many cases of \"one step forward, one\nstep back\".\n\nHope that helps,\nJonathan\n"},{"id":"238375","messageId":"vpqk3b56lik.fsf@anie.imag.fr","threadId":"36348","inReplyTo":"20140404172946.GI6851@google.com","subject":"Re: Patch Series v3 for \"use the $( ... ) construct for command substitution\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-04T17:40:35Z","receivedAt":"2014-04-04T17:40:35Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> If the script is \"obviously correct\" enough then there is no need\n> to manually go through 140 files after that point.\n\nThe script cannot be \"obviously correct\", as there are a lot of\npotential corner-cases (nested `, nesting ` within \", comments, ...).\n\n> If the only way to get this done is to actually manually review those\n> 140 files, I just don't think it's worth it.\n\nHonnestly, I went through the series once and it wasn't that painful. I\nneed to do a more carefull review, but using \"git diff --color-words=.\"\nit can be really fast.\n\nJunio suggested splitting the series into batches of around 10 patches,\nsending one per week, but that would make too many patches IMHO (14\nweeks ...).\n\nI'd suggest doing a first batch with only scripts that are not tests and\npushing this to git.git. Then the remaining series will be a bit less\nscary.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"238376","messageId":"20140404181215.GJ6851@google.com","threadId":"36348","inReplyTo":"vpqk3b56lik.fsf@anie.imag.fr","subject":"Re: Patch Series v3 for \"use the $( ... ) construct for command substitution\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-04T18:12:15Z","receivedAt":"2014-04-04T18:12:15Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> If the script is \"obviously correct\" enough then there is no need\n>> to manually go through 140 files after that point.\n>\n> The script cannot be \"obviously correct\", as there are a lot of\n> potential corner-cases (nested `, nesting ` within \", comments, ...).\n\nTo be a devil's advocate for a moment:\n\n * Comments are easy to detect.  Remember, we are not trying to handle\n   some adversary sending arbitrary well-formed shell scripts but just\n   the git source code which already follows a consistent style.  When\n   I search for /#.*/ in .sh files, every match except for\n\n\tt/test-lib-functions.sh:output=`echo; echo \"# Stderr is:\"; cat \"$stderr\"`\n\n   (which contains a backtick before the # mark) is a comment.\n\n * Nested ` is evil.  A search for the string \\' quickly finds them all,\n   and they are very very rare.\n\n   The only exception I see is git-svn tests, which independently of\n   everything else is an obvious thing to fix first.\n\n * Nesting `` within double-quotes has the same semantics as $()\n   within quotes.  I don't think that poses a problem.\n\n[...]\n>> If the only way to get this done is to actually manually review those\n>> 140 files, I just don't think it's worth it.\n>\n> Honnestly, I went through the series once and it wasn't that painful.\n\nIt is not just the people on the list reviewing now but others in the\nfuture wanting to understand whether it is safe to upgrade to a new\nversion or where a bug they have run into came from.  The simpler we\ncan make the task of convincing oneself that the patch behaves as\ndescribed, the better.\n\n140 patches worth of churn once every couple of years is not terrible,\nbut I really don't want to see this becoming a pattern. :/  And I\ndon't see how the upside in this example warrants it.\n\nHoping that clarifies,\nJonathan\n"},{"id":"238380","messageId":"xmqqa9c0q4i3.fsf@gitster.dls.corp.google.com","threadId":"36348","inReplyTo":"20140404181215.GJ6851@google.com","subject":"Re: Patch Series v3 for \"use the $( ... ) construct for command substitution\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-04T19:27:48Z","receivedAt":"2014-04-04T19:27:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> 140 patches worth of churn once every couple of years is not terrible,\n> but I really don't want to see this becoming a pattern. :/\n\nLikewise.\n\n> And I don't see how the upside in this example warrants it.\n\nParaphrasing http://article.gmane.org/gmane.linux.kernel/943020\n\nIt'd be good if we don't _add_ such things to the tree.\n\nBut IMO it's such a minor thing that once it _is_ in the tree, it's\nnot really worth the patch noise to go and fix it up.\n"}]}