From: Alexey Shumkin <alex.crezoff@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path
Date: Mon, 7 Sep 2015 14:05:54 +0300 [thread overview]
Message-ID: <20150907110554.GA482@dell-note> (raw)
In-Reply-To: <xmqqoahheqax.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 04, 2015 at 04:08:06PM -0700, Junio C Hamano wrote:
> Alexey Shumkin <alex.crezoff@gmail.com> writes:
>
> > Some repositories may have spaces in their paths. Currently `git-subtree`
> > raises an error in such cases.
> > Also, `git-subtree` currently does not have tests for its 'push' command.
> > Following patches are to fix these statements.
> >
> > Alexey Shumkin (2):
> > t7900-subtree: test the "space in a subdirectory name" case
> > contrib/subtree: respect spaces in a repository path
>
> Doesn't this order break bisection? It seems that you turn "subdir"
> to "sub dir" in existing tests, and I understand that the whole
> point of this series is that such a change will expose that the tool
> is broken, making tests fail.
It seems I have to reword commit messages to avoid such an interpretation.
Because, the first commit does not break anything. It is to change the
tests for `git-subtree` "to show" that `git-subtree`s already tested
commands (almost) work correctly if there are spaces in paths (except
--rejoin-msg issue).
And the second commit adds missing tests and the fix.
Should I add/commit the breaking test first and then commit the fix?
>
> Also, if you feel up to it, it might be a good idea to clean t7900
> test up to the current best practice before doing any other changes
> as a pure preparatory clean-up patch.
>
> Namely, using cd outside a subshell of the tests to move around is a
> bad thing to do, and you are adding more instance of it in this
> series. If one test with such a cd to go down fails before it has a
> chance to come back up (or go up and then fail to come back down),
> the later tests will be left in an unexpected place.
I understand this issue with "cd" (I've just followed the existing t7900
tests "code style").
>
> > contrib/subtree/git-subtree.sh | 4 +-
> > contrib/subtree/t/t7900-subtree.sh | 194 +++++++++++++++++++++++--------------
> > 2 files changed, 124 insertions(+), 74 deletions(-)
>
> Thanks.
--
Alexey Shumkin
E-mail: Alex.Crezoff@gmail.com
ICQ: 118001447
Jabber (GoogleTalk): Alex.Crezoff@gmail.com
Skype: crezoff
prev parent reply other threads:[~2015-09-07 11:06 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-04 22:24 [PATCH v1 0/2] contrib/subtree: make it respect spaces in a repository path Alexey Shumkin
2015-09-04 22:24 ` [PATCH v1 1/2] t7900-subtree: test the "space in a subdirectory name" case Alexey Shumkin
2015-09-04 22:24 ` [PATCH v1 2/2] contrib/subtree: respect spaces in a repository path Alexey Shumkin
2015-09-04 23:08 ` [PATCH v1 0/2] contrib/subtree: make it " Junio C Hamano
2015-09-07 11:05 ` Alexey Shumkin [this message]
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=20150907110554.GA482@dell-note \
--to=alex.crezoff@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.