I don't think my intuition matches that of other reviewers since my patches
seem to get a lot of review comments in this area.  That's why I'd like
those who have a preferred style preference to say specifically what they
want rather than point out exceptions individually.  If we cannot say how
these subject lines should be formatted with some specificity then it's
difficult to have objective review comments in this area.

On Tue, Dec 23, 2014 at 6:44 AM, Mike Holmes <[email protected]> wrote:
>
>
>
> On 23 December 2014 at 07:37, Bill Fischofer <[email protected]>
> wrote:
>
>> Thanks, Maxim.  I still think we should update CONTRIBUTING to clearly
>> specify how the project wants patch subject lines to be formatted, as well
>> as the other details I mentioned in the referenced post.  That will take
>> the guesswork out of the process.
>>
>
> I would say that you have the clearest view of what is not written down
> have just been through it, if you create a patch that answers your
> questions I think it would be right on the money for what is needed.
> It should be the ODP specifics only because there are a lot of good
> general write ups linked from contributing already and much of our audience
> will be familiar with it.
>
> My general rule for the patches, API, src  code style is to look at what
> everyone else has done and try to do it the same way.
>
>
>
>>
>> On Tue, Dec 23, 2014 at 2:28 AM, Maxim Uvarov <[email protected]>
>> wrote:
>>>
>>> Bill, Anderses point was that you are fixing example, not linux-generic.
>>> That is absolutely valid comment.
>>>
>>> I merge it with changing header to:
>>> example: timer: update timer test for new buffer pools
>>>
>>> Since it changes timer example:
>>> example/timer/odp_timer_test.c
>>>
>>> Maxim.
>>>
>>>
>>> On 12/20/2014 01:40 AM, Bill Fischofer wrote:
>>>
>>>> That's good to know, because the short log indeed does not say that.  :)
>>>>
>>>> Rather than commenting on these for each patch posted, why not update
>>>> the CONTRIBUTING file with a detailed summary of how ODP commit messages
>>>> should be formatted, including examples for the various common cases of
>>>> fixing bugs, adding features, performance enhancements, examples, tests,
>>>> etc.? Also include what should and should not be placed above and below the
>>>> s-o-b, again with examples illustrating correct form for the common cases.
>>>>
>>>> We can then all simply refer to this and use it as a model. This will
>>>> save a lot of noise on the review lists so that we can focus on reviewing
>>>> the code rather than how the code is packaged.
>>>>
>>>> Thanks.
>>>>
>>>> On Fri, Dec 19, 2014 at 3:27 PM, Anders Roxell <
>>>> [email protected] <mailto:[email protected]>> wrote:
>>>>
>>>>     The short log should not say:
>>>>     "example: buffer_timer_test: fix bad commit <short hash>"
>>>>
>>>>
>>>>
>>>>     On 19 December 2014 at 21:20, Bill Fischofer
>>>>     <[email protected] <mailto:[email protected]>>
>>>> wrote:
>>>>     > Signed-off-by: Bill Fischofer <[email protected]
>>>>     <mailto:[email protected]>>
>>>>
>>>>     > ---
>>>>     > Test was not properly updated to run with new buffer pool
>>>>     structure. Added
>>>>     > an explicit buffer number.
>>>>
>>>>     This good description should be above the s-o-b (Signed-off-by) =)
>>>>
>>>>     Cheers,
>>>>     Anders
>>>>
>>>>     >
>>>>     >  example/timer/odp_timer_test.c | 3 ++-
>>>>     >  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>     >
>>>>     > diff --git a/example/timer/odp_timer_test.c
>>>>     b/example/timer/odp_timer_test.c
>>>>     > index 0d6e31a..c754695 100644
>>>>     > --- a/example/timer/odp_timer_test.c
>>>>     > +++ b/example/timer/odp_timer_test.c
>>>>     > @@ -27,6 +27,7 @@
>>>>     >
>>>>     >  #define MAX_WORKERS           32            /**< Max worker
>>>>     threads */
>>>>     >  #define MSG_POOL_SIZE         (4*1024*1024) /**< Message pool
>>>>     size */
>>>>     > +#define MSG_NUM_BUFS          10000         /**< Number of
>>>>     timers */
>>>>     >
>>>>     >
>>>>     >  /** Test arguments */
>>>>     > @@ -316,7 +317,7 @@ int main(int argc, char *argv[])
>>>>     >
>>>>     >         params.buf_size  = 0;
>>>>     >         params.buf_align = 0;
>>>>     > -       params.num_bufs  = MSG_POOL_SIZE;
>>>>     > +       params.num_bufs  = MSG_NUM_BUFS;
>>>>     >         params.buf_type  = ODP_BUFFER_TYPE_TIMEOUT;
>>>>     >
>>>>     >         pool = odp_buffer_pool_create("msg_pool", shm, &params);
>>>>     > --
>>>>     > 1.8.3.2
>>>>     >
>>>>     > _______________________________________________
>>>>     > lng-odp mailing list
>>>>     > [email protected] <mailto:[email protected]>
>>>>     > http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> lng-odp mailing list
>>>> [email protected]
>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>
>>> *
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> [email protected]
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> [email protected]
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>
>
> --
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to