Dear VincentX,
please note in the subject what iteration your patch is, e. g. »[PATCH
1/2 v2]«. See `--subject-prefix` in `git help format-patch`.
Am Donnerstag, den 24.02.2011, 16:30 +0000 schrieb Becker, VincentX:
> From 55f84afd8575dadba697c746daa61ee07b333c57 Mon Sep 17 00:00:00 2001
> From: Vincent Becker <vincentx.bec...@intel.com>
> Date: Thu, 24 Feb 2011 16:52:05 +0100
> Subject: [PATCH] Add a new log target to a file descriptor in daemon
> configuration
> Signed-off-by: Vincent Becker <vincentx.bec...@intel.com>
> This patches adds the option to log pulseaudio messages into a file
> descriptor.
> The file descriptor can be a regular file or other type (character, block..).
> If the file given in parameter already exists, it is automatically renamed
> with current date infos.
You are missing a blank line after the commit summary (first line).
Applying your patch, e. g. with `git am …`, needs therefore manual
editing.
It is also common to add the Signed-off-by line at the end.
Could you add an example to the commit message about how to use the new
functionality so that people can easily test you new feature?
> ---
> src/daemon/cmdline.c | 98 +++++++++++++++++++++---------------------
> src/daemon/daemon-conf.c | 108
> +++++++++++++++++++++++++++++++++++++++++++++-
> src/pulsecore/log.c | 6 +++
> src/pulsecore/log.h | 5 ++
> 4 files changed, 166 insertions(+), 51 deletions(-)
>
> diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
> index f6cdcdc..7f72b8c 100644
> --- a/src/daemon/cmdline.c
> +++ b/src/daemon/cmdline.c
> @@ -114,59 +114,59 @@ void pa_cmdline_help(const char *argv0) {
>
> printf(_("%s [options]\n\n"
> "COMMANDS:\n"
> - " -h, --help Show this help\n"
> - " --version Show version\n"
> - " --dump-conf Dump default
> configuration\n"
> - " --dump-modules Dump list of available
> modules\n"
> - " --dump-resample-methods Dump available resample
> methods\n"
> - " --cleanup-shm Cleanup stale shared
> memory segments\n"
> - " --start Start the daemon if it
> is not running\n"
> - " -k --kill Kill a running daemon\n"
> - " --check Check for a running
> daemon (only returns exit code)\n\n"
> + " -h, --help Show this
> help\n"
> + " --version Show
> version\n"
> + " --dump-conf Dump default
> configuration\n"
> + " --dump-modules Dump list of
> available modules\n"
> + " --dump-resample-methods Dump
> available resample methods\n"
> + " --cleanup-shm Cleanup stale
> shared memory segments\n"
> + " --start Start the
> daemon if it is not running\n"
> + " -k --kill Kill a
> running daemon\n"
> + " --check Check for a
> running daemon (only returns exit code)\n\n"
>
> "OPTIONS:\n"
> - " --system[=BOOL] Run as system-wide
> instance\n"
> - " -D, --daemonize[=BOOL] Daemonize after
> startup\n"
> - " --fail[=BOOL] Quit when startup
> fails\n"
> - " --high-priority[=BOOL] Try to set high nice
> level\n"
> - " (only available as root,
> when SUID or\n"
> - " with elevated
> RLIMIT_NICE)\n"
> - " --realtime[=BOOL] Try to enable realtime
> scheduling\n"
> - " (only available as root,
> when SUID or\n"
> - " with elevated
> RLIMIT_RTPRIO)\n"
> - " --disallow-module-loading[=BOOL] Disallow module user
> requested module\n"
> - " loading/unloading after
> startup\n"
> - " --disallow-exit[=BOOL] Disallow user requested
> exit\n"
> - " --exit-idle-time=SECS Terminate the daemon
> when idle and this\n"
> - " time passed\n"
> - " --module-idle-time=SECS Unload autoloaded
> modules when idle and\n"
> - " this time passed\n"
> - " --scache-idle-time=SECS Unload autoloaded
> samples when idle and\n"
> - " this time passed\n"
> - " --log-level[=LEVEL] Increase or set
> verbosity level\n"
> - " -v Increase the verbosity
> level\n"
> - " --log-target={auto,syslog,stderr} Specify the log target\n"
> - " --log-meta[=BOOL] Include code location in
> log messages\n"
> - " --log-time[=BOOL] Include timestamps in
> log messages\n"
> - " --log-backtrace=FRAMES Include a backtrace in
> log messages\n"
> - " -p, --dl-search-path=PATH Set the search path for
> dynamic shared\n"
> - " objects (plugins)\n"
> - " --resample-method=METHOD Use the specified
> resampling method\n"
> - " (See
> --dump-resample-methods for\n"
> - " possible values)\n"
> - " --use-pid-file[=BOOL] Create a PID file\n"
> - " --no-cpu-limit[=BOOL] Do not install CPU load
> limiter on\n"
> - " platforms that support
> it.\n"
> - " --disable-shm[=BOOL] Disable shared memory
> support.\n\n"
> + " --system[=BOOL] Run as
> system-wide instance\n"
> + " -D, --daemonize[=BOOL] Daemonize
> after startup\n"
> + " --fail[=BOOL] Quit when
> startup fails\n"
> + " --high-priority[=BOOL] Try to set
> high nice level\n"
> + " (only
> available as root, when SUID or\n"
> + " with elevated
> RLIMIT_NICE)\n"
> + " --realtime[=BOOL] Try to enable
> realtime scheduling\n"
> + " (only
> available as root, when SUID or\n"
> + " with elevated
> RLIMIT_RTPRIO)\n"
> + " --disallow-module-loading[=BOOL] Disallow
> module user requested module\n"
> + "
> loading/unloading after startup\n"
> + " --disallow-exit[=BOOL] Disallow user
> requested exit\n"
> + " --exit-idle-time=SECS Terminate the
> daemon when idle and this\n"
> + " time passed\n"
> + " --module-idle-time=SECS Unload
> autoloaded modules when idle and\n"
> + " this time
> passed\n"
> + " --scache-idle-time=SECS Unload
> autoloaded samples when idle and\n"
> + " this time
> passed\n"
> + " --log-level[=LEVEL] Increase or
> set verbosity level\n"
> + " -v Increase the
> verbosity level\n"
> + " --log-target={auto,syslog,stderr,FILE PATH} Specify the
> log target\n"
> + " --log-meta[=BOOL] Include code
> location in log messages\n"
> + " --log-time[=BOOL] Include
> timestamps in log messages\n"
> + " --log-backtrace=FRAMES Include a
> backtrace in log messages\n"
> + " -p, --dl-search-path=PATH Set the
> search path for dynamic shared\n"
> + " objects
> (plugins)\n"
> + " --resample-method=METHOD Use the
> specified resampling method\n"
> + " (See
> --dump-resample-methods for\n"
> + " possible
> values)\n"
> + " --use-pid-file[=BOOL] Create a PID
> file\n"
> + " --no-cpu-limit[=BOOL] Do not
> install CPU load limiter on\n"
> + " platforms
> that support it.\n"
> + " --disable-shm[=BOOL] Disable
> shared memory support.\n\n"
>
> "STARTUP SCRIPT:\n"
> - " -L, --load=\"MODULE ARGUMENTS\" Load the specified
> plugin module with\n"
> - " the specified argument\n"
> - " -F, --file=FILENAME Run the specified
> script\n"
> - " -C Open a command line on
> the running TTY\n"
> - " after startup\n\n"
> + " -L, --load=\"MODULE ARGUMENTS\" Load the
> specified plugin module with\n"
> + " the specified
> argument\n"
> + " -F, --file=FILENAME Run the
> specified script\n"
> + " -C Open a
> command line on the running TTY\n"
> + " after
> startup\n\n"
>
> - " -n Don't load default
> script file\n"),
> + " -n Don't load
> default script file\n"),
> pa_path_get_filename(argv0));
> }
Could you add that indentation change in a separate patch? It makes it
easier to review in my opinion. It is not necessary though.
[…]
Could you also document the new option in the manual and add an example
to it? Do the config file templates also need to be updated?
Thanks,
Paul
signature.asc
Description: This is a digitally signed message part
_______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
