Re: [PATCH 1/2] Simplify tracing code by removing trace key normalization concept

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gennady Kupava <gennady.kupava@xxxxxxxxx> writes:

>> The usual style comment on the subject applies here.
>
> Oh sure, 50 characters. 'Remove trace key normalization concept' would
> be better?

I was referring to #summary-section of Documentation/SubmittingPatches

    The first line of the commit message should be a short description (50
    characters is the soft limit, see DISCUSSION in linkgit:git-commit[1]),
    and should skip the full stop.  It is also conventional in most cases to
    prefix the first line with "area: " where the area is a filename or
    identifier for the general area of the code being modified, e.g.

    * doc: clarify distinction between sign-off and pgp-signing
    * githooks.txt: improve the intro section

    If in doubt which identifier to use, run `git log --no-merges` on the
    files you are modifying to see the current conventions.

    [[summary-section]]
    It's customary to start the remainder of the first line after "area: "
    with a lower-case letter. E.g. "doc: clarify...", not "doc:
    Clarify...", or "githooks.txt: improve...", not "githooks.txt:
    Improve...".

> So comments explaining that, while implementing trace optimization, I
> saw two options:
> 1. Move normalization function from .c file to .h file
> 2. Remove it
>
> And why I choose removal - not used, would complicate header without
> any benefit, and actually I didn't mention minor benefit of
> compilation speed. This trace.h included and used in lots of places so
> it will take compiler some time to actually eliminate the code.
>
>> Puzzled.
>
> Does it make more sense now?

The thought behind the change flows much better in the above
explanation than your four-bullet list (which a reader would often
assume are parallel and orthogonal).  "Remove this, because it is
not used" is the primary thing for this step, and the fact that the
later step benefit from that change, while it may be more important
to the person who want to see that later change, is incidental to
see if this change makes sense as a standalone patch.

And that primary thing, "remove this, because it is not used and
only complicates the code" is what we want to start the log message
with.  The first sentence ("this needs to be moved there") saying
what the patch chose not to was the source of the confusion.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux