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