You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2016/04/14 00:25:38 UTC

Re: Review Request 46027: Documented `libprocess` helper function.

It's not a hard requirement anymore, I'm asking to maintain consistency. We
originally had the 70 comment character limit to improve readability but it
was relaxed because clang-format wasn't able to distinguish line length
between code and comments. In retrospect, relaxing this has led to some
unfortunate inconsistencies in the code ( see
http://markmail.org/message/ptwqrehbpff4eefa ). l think there's value in
wrapping to reduce jaggedness and wrapping at 70, so ideally we can get
back to 70 if there's support in clang-format. For now I would be in favor
of adding back the style guidelines, even though the style checker can't
catch these: http://markmail.org/message/ptwqrehbpff4eefa

On Wed, Apr 13, 2016 at 2:40 PM, Neil Conway <ne...@gmail.com> wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46027/
>
> On April 13th, 2016, 9:33 p.m. UTC, *Ben Mahler* wrote:
>
> 3rdparty/libprocess/src/process.cpp
> <https://reviews.apache.org/r/46027/diff/2/?file=1340142#file1340142line559> (Diff
> revision 2)
>
> static void transport(Message* message, ProcessBase* sender = NULL)
>
> 559
>
> // Returns true if `request` contains an inbound libprocess message.  A
>
> s/.  A/. A/
>
> Thanks for clarifying, sorry for the confusion!
>
> On April 13th, 2016, 9:37 p.m. UTC, *Ben Mahler* wrote:
>
> Also, would you mind wrapping comments at 70 to stay consistent with most of our existing code?
>
> // Returns true if `request` contains an inbound libprocess message.// A libprocess message can either be sent by another instance of// libprocess (i.e. both of the "User-Agent" and "Libprocess-From"// headers will be set), or a client that speaks the libprocess// protocol (i.e. only the "Libprocess-From" header will be set).// This function returns true for either case.
>
> Per commit f9c2604ea97b91f8a9ec3b2863317761679b1c86, I thought that wasn't a requirement anymore?
>
>
> - Neil
>
> On April 12th, 2016, 12:23 a.m. UTC, Neil Conway wrote:
> Review request for mesos and Ben Mahler.
> By Neil Conway.
>
> *Updated April 12, 2016, 12:23 a.m.*
> *Repository: * mesos
> Description
>
> This is a bit confusing, because different libprocess HTTP
> clients set different headers.
>
> Diffs
>
>    - 3rdparty/libprocess/src/process.cpp
>    (5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a)
>
> View Diff <https://reviews.apache.org/r/46027/diff/>
>