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.becker at 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.becker at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20110224/b3201b7f/attachment.pgp>
