[pulseaudio-discuss] [PATCH v3 00/12] Add module-gsettings

[Tanu Kaskinen]

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