On Wed, 28 Aug 2019 11:57:42 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> From: Tzvetomir Stoyanov <tstoyanov@xxxxxxxxxx>
Patch looks good. Just need some fixes in the change log.
Subject: "trace-cmd: Refactored some functions in trace-record.c"
>
> In order to reuse the code inside the trace-cmd application, few
"a few"
> functions from trace-record.c are refactored:
> - make_instances() and tracecmd_remove_instances() are splited.
"are split"
> New ones are created: tracecmd_make_instance() and tracecmd_remove_instance(),
"Following functions were added to export static functionality:
tracecmd_make_instance()
tracecmd_remove_instance()"
> which are visible outside the trace-record.c
"outside of trace-record.c"
> - Following functions are made non-static: tracecmd_init_instance()
> get_instance_dir(), write_instance_file(), write_tracing_on(),
> tracecmd_set_clock()
Make the above more a visible list.
- The following functions are made non-static:
tracecmd_init_instance()
get_instance_dir()
write_instance_file()
write_tracing_on()
tracecmd_set_clock()
And don't be afraid to add empty lines between these bullet points. It
makes it easier to read. Whitespace can be your friend ;-)
> - New function is implemented: tracecmd_local_cpu_count(), an internal
> API to get local_cpu_count.
Turn the above into:
- Created a new function:
tracecmd_local_cpu_count() - an internal API to get the
local_cpu_count.
Note, if you do bullet lists, please keep everything aligned, and space
out the bullet list.
Instead of:
>>>>
- Following functions are made non-static: tracecmd_init_instance()
get_instance_dir(), write_instance_file(), write_tracing_on(),
tracecmd_set_clock()
- New function is implemented: tracecmd_local_cpu_count(), an internal
API to get local_cpu_count.
<<<<
Have:
>>>>
- Following functions are made non-static: tracecmd_init_instance()
get_instance_dir(), write_instance_file(), write_tracing_on(),
tracecmd_set_clock()
- New function is implemented: tracecmd_local_cpu_count(), an internal
API to get local_cpu_count.
<<<<
Much easier to read.
Proper spacing is critical for change logs. It makes things stand out
much easier when scanning a "git log" looking for something specific.
When there's a lot of text bound together, it's much harder to find
something unless you know exactly what to search for via a search
string.
Thanks!
-- Steve
>
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@xxxxxxxxxx>
> ---
> tracecmd/include/trace-local.h | 9 ++++
> tracecmd/trace-record.c | 88 +++++++++++++++++++---------------
> 2 files changed, 59 insertions(+), 38 deletions(-)
>
|