Mojo: Move the header bits of MessageInTransit into a separate struct.

Mojo: Move the header bits of MessageInTransit into a separate struct.

This is a small reorganization before a bigger one.

R=yzshen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252082

[viettrungluu, 2014-02-19 18:10:32]

On second thought, don't look at this yet.

[yzshen1, 2014-02-19 18:14:04]

Sure. Please ping me when the CL is ready again.


On Wed, Feb 19, 2014 at 10:10 AM, <viettrungluu@chromium.org> wrote:

> On second thought, don't look at this yet.
>
> https://codereview.chromium.org/163023004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[viettrungluu, 2014-02-19 18:14:31]

New version up. :)

[yzshen1, 2014-02-19 18:35:06]

LGTM with one nit

https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
File mojo/system/message_in_transit.h (right):

https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
mojo/system/message_in_transit.h:105: Header(uint32_t data_size, Type type,
Subtype subtype, EndpointId source_id,
I think we need to do each parameter on a separate line for the parameter list
and the initializer list if they don't fit in a single line.

[viettrungluu, 2014-02-19 18:39:17]

On 2014/02/19 18:35:06, yzshen1 wrote:
> LGTM with one nit
> 
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> File mojo/system/message_in_transit.h (right):
> 
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> mojo/system/message_in_transit.h:105: Header(uint32_t data_size, Type type,
> Subtype subtype, EndpointId source_id,
> I think we need to do each parameter on a separate line for the parameter list
> and the initializer list if they don't fit in a single line.

Strictly speaking, I believe that the original Google style guide allows what I
did. We may have amended the Chromium style guide to disallow it, since we never
do it in Chromium. But I don't want to consume the vertical space (for something
embedded in a header file). :P

[viettrungluu, 2014-02-19 18:41:21]

On 2014/02/19 18:39:17, viettrungluu wrote:
> On 2014/02/19 18:35:06, yzshen1 wrote:
> > LGTM with one nit
> > 
> >
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> > File mojo/system/message_in_transit.h (right):
> > 
> >
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> > mojo/system/message_in_transit.h:105: Header(uint32_t data_size, Type type,
> > Subtype subtype, EndpointId source_id,
> > I think we need to do each parameter on a separate line for the parameter
list
> > and the initializer list if they don't fit in a single line.
> 
> Strictly speaking, I believe that the original Google style guide allows what
I
> did. We may have amended the Chromium style guide to disallow it, since we
never
> do it in Chromium. But I don't want to consume the vertical space (for
something
> embedded in a header file). :P

(Actually, the public Google C++ style guide is at least a little vague on this.
And it isn't clarified for Chromium.)

[yzshen1, 2014-02-19 19:25:51]

On 2014/02/19 18:41:21, viettrungluu wrote:
> On 2014/02/19 18:39:17, viettrungluu wrote:
> > On 2014/02/19 18:35:06, yzshen1 wrote:
> > > LGTM with one nit
> > > 
> > >
> >
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> > > File mojo/system/message_in_transit.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/163023004/diff/80001/mojo/system/message_in_t...
> > > mojo/system/message_in_transit.h:105: Header(uint32_t data_size, Type
type,
> > > Subtype subtype, EndpointId source_id,
> > > I think we need to do each parameter on a separate line for the parameter
> list
> > > and the initializer list if they don't fit in a single line.
> > 
> > Strictly speaking, I believe that the original Google style guide allows
what
> I
> > did. We may have amended the Chromium style guide to disallow it, since we
> never
> > do it in Chromium. But I don't want to consume the vertical space (for
> something
> > embedded in a header file). :P
> 
> (Actually, the public Google C++ style guide is at least a little vague on
this.
> And it isn't clarified for Chromium.)

Chromium does have a rule for function declaration and definition:
"""For function declarations and definitions, put each argument on a separate
line unless the whole declaration fits on one line."""

WRT initializer list, the Google style guide is a little vague as you said:
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

[viettrungluu, 2014-02-19 21:02:10]

Committed patchset #2 manually as r252082 (presubmit successful).