threads / patch / 47495

patchFix confusing wording

Subject: [PATCH] Fix confusing wording

## tl;dr

3 messages between Dec 25, 2017 and Jan 7, 2018. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ivan Pozdeev· Dec 25, 2017, 10:26 UTC · lore

Not sure if I should add a CVE-2009-0037 reference as well.

--- http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to http.c +1 −1

diff --git a/http.c b/http.c
index 215bebe..26b3386 100644
--- a/http.c
+++ b/http.c
@@ -802,7 +802,7 @@ static CURL *get_curl_handle(void)
get_curl_allowed_protocols(-1));
#else
warning("protocol restrictions not applied to curl redirects because\n"
- "your curl version is too old (>= 7.19.4)");
+ "your libcurl version is too old (< 7.19.4)");
#endif
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);

-- 
2.10.0.windows.1

-- 
Regards,
Ivan

Junio C Hamano· Dec 27, 2017, 19:44 UTC · re: Ivan Pozdeev · lore

Re: [PATCH] Fix confusing wording

Ivan Pozdeev <vano@mail.mipt.ru> writes:

> Not sure if I should add a CVE-2009-0037 reference as well.

Not in an end-user facing message like this one, I would say.

>
> ---

Sign off?

Show 14 quoted lines

> http.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index 215bebe..26b3386 100644
> --- a/http.c
> +++ b/http.c
> @@ -802,7 +802,7 @@ static CURL *get_curl_handle(void)
> get_curl_allowed_protocols(-1));
> #else
> warning("protocol restrictions not applied to curl redirects because\n"
> - "your curl version is too old (>= 7.19.4)");
> + "your libcurl version is too old (< 7.19.4)");
> #endif

Between 'curl' and 'libcurl', the latter certainly is more technically correct. I have a feeling that it would be better to spell out ">=" as "requires at least" than replacing it with equally cryptic "<" (it is OK to spell it out as "your libcurl version is older than minimum required 7.19.4", too).

Thanks.

Ivan Pozdeev· Jan 7, 2018, 16:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix confusing wording

On 27.12.2017 22:44, Junio C Hamano wrote:

> Ivan Pozdeev <vano@mail.mipt.ru> writes:
>
>> Not sure if I should add a CVE-2009-0037 reference as well.
> Not in an end-user facing message like this one, I would say.

To me, as an end user, the current message had no meaning at all. It even sounds like I'm better off for having an old version 'cuz git tried to apply some limitation to the download, and couldn't. If it had "see <CVE> for more details", I could quickly find out the message's real meaning. Currently, I have to `git blame' the source to do that -- certainly something that few users would resort to.

>> ---
> Sign off?

https://github.com/native-api/git/blob/master/Documentation/SubmittingPatches wasn't clear that this is required (TortoiseGit doesn't add it by default). Do I need to remake the patch or something?

Show 19 quoted lines

>> http.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/http.c b/http.c
>> index 215bebe..26b3386 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -802,7 +802,7 @@ static CURL *get_curl_handle(void)
>> get_curl_allowed_protocols(-1));
>> #else
>> warning("protocol restrictions not applied to curl redirects because\n"
>> - "your curl version is too old (>= 7.19.4)");
>> + "your libcurl version is too old (< 7.19.4)");
>> #endif
> Between 'curl' and 'libcurl', the latter certainly is more
> technically correct.  I have a feeling that it would be better to
> spell out ">=" as "requires at least" than replacing it with equally
> cryptic "<" (it is OK to spell it out as "your libcurl version is
> older than minimum required 7.19.4", too).

I don't see how "your version is too old (less that <version>)" is unclear. Any wording with "required" would be incorrect because a newer version isn't really "required", git can work with an older one, too.

> Thanks.

You're welcome :-)

-- 
Regards,
Ivan