Thank you so much, David!
On Thu, Feb 4, 2016 at 5:44 AM, David Drysdale <drysdale_at_google.com> wrote:
> OK, I've added that change to the repo (and put in a quick test case for
> it).
>
> The Travis builds are running, but be warned: the coverage-generation
> build may well fail because coveralls.io is flaky at the moment.
>
> D.
>
> On Wed, Feb 3, 2016 at 8:03 PM, Fedor Indutny <fedor_at_indutny.com> wrote:
>
>> My bad, didn't run the code indeed. All fixed, code tested in:
>>
>> https://github.com/nodejs/node/pull/5062
>> https://ci.nodejs.org/job/node-test-commit/2081/
>>
>> Appears to be working just fine with node.js .
>>
>> Updated patch in attachment.
>>
>> Sorry about typos!
>>
>> Thank you,
>> Fedor.
>>
>> On Wed, Feb 3, 2016 at 10:48 AM, David Drysdale <drysdale_at_google.com>
>> wrote:
>>
>>> Still seems the name of the new function is inconsistent:
>>> - the code and manpage have: ares_parse_txt_reply_ext
>>> - the master ares.h header file has: ares_parse_txt_ext
>>> - the commit message has: ares_parse_txt_reply_ex
>>>
>>> Have you run this code?
>>>
>>> D.
>>>
>>>
>>> On Mon, Feb 1, 2016 at 11:10 PM, Fedor Indutny <fedor_at_indutny.com>
>>> wrote:
>>>
>>>> Thank you, Daniel.
>>>>
>>>> Hopefully, I have fixed all mentioned nits. I tried to do my best on
>>>> the documentation, but please forgive me some grammar issues since I am not
>>>> a native english speaker.
>>>>
>>>> If you have any other suggestions with regards to either code, docs, or
>>>> both. I will be more than happy to put them into the patch.
>>>>
>>>> Thanks again,
>>>> Fedor.
>>>>
>>>> On Mon, Feb 1, 2016 at 5:29 PM, Daniel Stenberg <daniel_at_haxx.se> wrote:
>>>>
>>>>> On Mon, 1 Feb 2016, Fedor Indutny wrote:
>>>>>
>>>>> I have made fixes to the patch from 2015-03-21, according to your
>>>>>> comments.
>>>>>>
>>>>>> Please take a look.
>>>>>>
>>>>>
>>>>> Two comments from me:
>>>>>
>>>>> 1. Please don't refer to source code file names in ares.h. That's a
>>>>> public header file and users of c-ares will read that and have no idea
>>>>> about the specific source file or what we're talking about. You could of
>>>>> course still explain that the new struct is a superset of the old one.
>>>>>
>>>>> 2. Regarding the documentation/functionality. The new man page
>>>>> paragraph says:
>>>>>
>>>>> "Continuations are linked using next pointer field in the structure"
>>>>>
>>>>> (I think it should say "*the* next pointer...")
>>>>>
>>>>> ... but it doesn't really explain how it works with any details.
>>>>> Should the user then consider the 'txt' areas to be joined? I don't think
>>>>> we can presume users will understand what a continuation is if we don't
>>>>> explain it!
>>>>>
>>>>> --
>>>>>
>>>>> / daniel.haxx.se
>>>>>
>>>>
>>>>
>>>
>>
>
Received on 2016-02-04