On Thu, 2018-04-19 at 13:18 +0200, Georg Chini wrote: > On 19.04.2018 13:06, Tanu Kaskinen wrote: > > On Wed, 2018-04-18 at 19:29 +0200, Georg Chini wrote: > > > On 17.04.2018 08:07, Tanu Kaskinen wrote: > > > > Here are the PulseAudio patches for the GConf -> GSettings migration. > > > > The first patch is the original patch from Sylvain Baubeau, rebased on > > > > current master, and the rest of the patch set are various fixups > > > > (mostly very simple stuff). > > > > > > > > The first patch doesn't necessarily need to be reviewed, since I have > > > > already reviewed it, but note that stdin-util.c and .h don't follow the > > > > usual style conventions. I decided not to fix the style, because the > > > > code will hopefully be removed some day anyway (when moving from > > > > GSettings to the native protocol, which requires adding the necessary > > > > features to the native protocol), but if someone has a problem with > > > > that, then I can fix the style issues as well. > > > > > > > > Sylvain Baubeau (1): > > > > module-gsettings: new module to store configuration using gsettings > > > > > > > > Tanu Kaskinen (11): > > > > .gitignore: add module-gsettings related things > > > > default.pa: add module-gsettings > > > > gsettings: add the modules schema to the schema description file > > > > gconf, gsettings: fix config.h includes > > > > gsettings: rename "module" to "module-group" > > > > build-sys: remove a redundant enable_gsettings check > > > > gsettings: check that children haven't been deleted before using them > > > > gsettings: remove bad signal connection > > > > gsettings: free the module-group GSettings objects after use > > > > gsettings: free group_names after use > > > > build-sys: enable GSettings by default > > > > > > > > configure.ac | 34 ++- > > > > src/.gitignore | 2 + > > > > src/Makefile.am | 31 ++- > > > > src/daemon/default.pa.in | 13 + > > > > src/modules/gconf/module-gconf.c | 290 +-------------------- > > > > src/modules/gsettings/gsettings-helper.c | 130 +++++++++ > > > > src/modules/gsettings/module-gsettings.c | 114 ++++++++ > > > > .../org.freedesktop.pulseaudio.gschema.xml | 115 ++++++++ > > > > src/modules/stdin-util.c | 279 ++++++++++++++++++++ > > > > src/modules/stdin-util.h | 85 ++++++ > > > > 10 files changed, 803 insertions(+), 290 deletions(-) > > > > create mode 100644 src/modules/gsettings/gsettings-helper.c > > > > create mode 100644 src/modules/gsettings/module-gsettings.c > > > > create mode 100644 src/modules/gsettings/org.freedesktop.pulseaudio.gschema.xml > > > > create mode 100644 src/modules/stdin-util.c > > > > create mode 100644 src/modules/stdin-util.h > > > > > > > > > > Patches look all good to me. One remark to the first patch: Should > > > the copyright notice in the new files not be removed? > > > > Well, we don't have any rule to avoid adding new copyright notices. I > > don't personally really care. > > > > If you want to have that kind of rule, I think we should then remove > > all copyright notices (except maybe those that are in files that are > > copied from external projects). > > > > Thanks for the review, I'll push these. I'll still post one more patch > > that moves the gsettings-data-convert call from paprefs to pulseaudio. > > > > Well, in my patches you complained about the " Copyright > Lennard Poettering 2006" in the preamble of new files. > But I don't care, so leave it as it is. module-gsettings.c is extremely close to module-gconf.c (only three lines have changed, and the changes are trivial), so copying the copyright notice makes sense. The stdin-util files are mostly from old module-gconf.c too. gsettings-helper.c, on the other hand, shouldn't be attributed to Lennart. I'll remove the copyright notice from that file. -- Tanu https://liberapay.com/tanuk https://www.patreon.com/tanuk