Hey, thanks for the feedback. I included the changes in the updated patch.
Best regards
Luca
On 11.09.19, 08:52, "Willy Tarreau" <w...@1wt.eu> wrote:
On Tue, Sep 10, 2019 at 03:56:43PM +0200, Tim Düsterhus wrote:
> Luca,
>
> Am 10.09.19 um 15:48 schrieb Schimweg, Luca:
> > sorry, didn't know about these guidelines. I packed all changes into
one commit in the patch file in the attachments.
>
> One more thing: Do not leave out the body of the commit message. See the
> paragraph starting line 562 of the CONTRIBUTING file:
>
>
https://github.com/haproxy/haproxy/blob/e058f7359f3822beb8552f77a6d439cb053edb3f/CONTRIBUTING#L562-L567
>
> > I included your changes, you are right, the code looks much better now.
>
> I spotted a little typo:
>
> ((rnd[1] >> 16u) % 0xFFF) | 0x4000, // highest 4 bits indicate the uuid
> version
>
> It should read `&` instead of `%` there.
>
> > I'm open for any feedback.
>
> I suggest to wait for Willy before sending an updated patch. I guess he
> has some more feedback.
Thank to you both for the patch and the review. I agree with everything
Tim said. I have oe other minor comment which is to add an argument check
function to verify that the argument, if present, is 4, and default it to
4 when absent. Have a look at sample_conv_protobuf_check() and
smp_check_concat() for example. Otherwise I'm fine with the patch.
Cheers,
Willy
0001-MINOR-sample-Add-UUID-fetch.patch
Description: 0001-MINOR-sample-Add-UUID-fetch.patch
