Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Apr 18, 2013, 10:02 UTC
- Message-ID
- <CAMP44s35tmmgNd-vJQnqG56avSPY2NOBR6iR7eNtpfemXsfPJA@mail.gmail.com>
- In-Reply-To
- <20130418094531.GU2278@serenity.lan>
On Thu, Apr 18, 2013 at 4:45 AM, John Keeping <john@keeping.me.uk> wrote:
> On Thu, Apr 18, 2013 at 04:27:57AM -0500, Felipe Contreras wrote:
Show 12 quoted lines
>> Maybe my version was "misleading" because it didn't mention it was for >> historical reasons, but yours is plain wrong; remote helpers might not >> be using 'import' or 'export' at all, so not all remote helpers should >> always specify an explicit refspec. And if the previous text "It is >> recommended that all importers providing the 'import' capability use >> this. It's mandatory for 'export'." does not convey the idea these >> remote helpers should always specify an explicit refspec, I don't know >> what it does. > > I didn't say mine was correct, but there was a reason that I wanted to > change the existing text. Just going back to what was there before is > not a good way to make things better.
And just because it was that way before doesn't mean it was worst. Your patch essentially switched from "it is implied", to "it should be explicit", which is wrong. Going back to the previous text is the simplest change that restores a reasonable explanation. The next patches in this series deal with the rest of the issues in this text.
> In my copy of pu I don't see the text "It's mandatory for 'export'", it > just stops after "It is recommended that all importers providing the > 'import' capability use this". That paragraph also starts with "This > modifies the 'import' capability" making no mention of export.
This is patch 1 of 6, keep going.
-- Felipe Contreras