Date: Fri, 15 May 2015 10:03:30 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Wang Nan <wangnan0@huawei.com>
Cc: paulus@samba.org, a.p.zijlstra@chello.nl, mingo@redhat.com,
        acme@kernel.org, namhyung@kernel.org, jolsa@kernel.org,
        adrian.hunter@intel.com, dsahern@gmail.com, ast@plumgrid.com,
        daniel@iogearbox.net, brendan.d.gregg@gmail.com,
        masami.hiramatsu.pt@hitachi.com, linux-kernel@vger.kernel.org,
        lizefan@huawei.com, pi3orama@163.com
Subject: Re: [RFC PATCH v2 00/37] perf tools: introduce 'perf bpf' command to
 load eBPF programs.
Message-ID: <20150515080330.GA5277@gmail.com>
References: <1431676290-1230-1-git-send-email-wangnan0@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1431676290-1230-1-git-send-email-wangnan0@huawei.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3182
Lines: 80


Just a small stylistic side note:

> Wang Nan (37):
>   tools perf: set vmlinux_path__nr_entries to 0 in vmlinux_path__exit.
>   tools lib traceevent: install libtraceevent.a into libdir.
>   tools build: Allow other override features to check.
>   tools include: add __aligned_u64 to types.h.
>   tools lib bpf: introduce 'bpf' library to tools.
>   tools lib bpf: allow set printing function.
>   tools lib bpf: defines basic interface.
>   tools lib bpf: open eBPF object file and do basic validation.
>   tools lib bpf: check swap according to EHDR.
>   tools lib bpf: iterater over elf sections to collect information.
>   tools lib bpf: collect version and license from ELF.
>   tools lib bpf: collect map definitions.
>   tools lib bpf: collect config section in object.
>   tools lib bpf: collect symbol table in object files.
>   tools lib bpf: collect bpf programs from object files.
>   tools lib bpf: collect relocation sections from object file.
>   tools lib bpf: collect relocation instructions for each program.
>   tools lib bpf: clean elf memory after loading.
>   tools lib bpf: add bpf.c/h for common bpf operations.
>   tools lib bpf: create maps needed by object file.
>   tools lib bpf: relocation programs.
>   tools lib bpf: introduce bpf_load_program to bpf.c.
>   tools lib bpf: load bpf programs in object file into kernel.
>   tools lib bpf: accessors of bpf_program.
>   tools lib bpf: accessors for struct bpf_object.
>   tools perf: Add new 'perf bpf' command.
>   tools perf: make perf depend on libbpf.
>   tools perf: add 'perf bpf record' subcommand.
>   tools perf: add bpf-loader and open elf object files.
>   tools perf: collect all bpf programs.
>   tools perf: config probe points of eBPF programs during prepartion.
>   tools perf bpf: probe at kprobe points.
>   tools perf bpf: load eBPF object into kernel.
>   tools perf: add a bpf_wrapper global flag.
>   tools perf: add bpf_fd field to evsel and introduce new event syntax.
>   tools perf: generate event argv.
>   tools perf bpf: passes generated arguments to cmd_record.

The titles of the changes have numerous problems and inconsistencies:

 - use consistent capitalization, i.e.:

    tools perf: Collect all bpf programs

 - don't use period at the end, i.e.:

    tools perf: Generate event argv

 - use consistent present tense verbs, i.e.:

    -  tools lib bpf: defines basic interface.
    +  tools lib bpf: Define basic interface

 - Always use verbs! I.e. these are bad:

    -   tools perf: config probe points of eBPF programs during prepartion.
    -   tools lib bpf: relocation programs.

 - take a look at 'git log tools/perf' to see what the established 
   title style is. For example it's not 'tools perf' but 'perf tools', 
   etc.

etc.

There's not a single title in this series that has a proper title. 
This makes the shortlog a difficult read and gives a bad first 
impression. Please fix.

Thanks,

    Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/