GNOME Bugzilla – Bug 731799
add metainfo files for zeitgeist plugin
Last modified: 2014-06-21 15:01:55 UTC
In bug 731632 I've added metainfo for all plugins which in gedit-plugins project. I think we should add metainfo for plugins in gedit. You probably know that Fedora maintainers of gedit splitted it out to gedit and gedit-zeitgeist. I think in the near future we will split it to many packages. Richard today added feature for projects which have plugins as a part of main program. appstream_builder (was: createrepo_as) building AppStream metadata for each distro. If project has AppData and MetaInfo at the same time it will work this way: * if distro has appdata and metainfo in the same package - it just ignore metainfo and provides for software center only main package. * if distro has appdata and metainfo in separate packages (e.g. gedit and gedit-zeigeist), them will shown in software center as main package and plugin for it. I'm written patch, so review it please.
Created attachment 278627 [details] [review] zeitgeist: add MetaInfo file Reference: https://bugzilla.gnome.org/show_bug.cgi?id=731799 Signed-off-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Created attachment 278628 [details] [review] zeitgeist: add MetaInfo file fix makefile.am and add to POTFILES.in
I think we should move the zeitgeist plugin to gedit-plugins: it is not very used and many distros split it out due to the extra dependency. For the core plugins please do not split them to separate packages, they are core part of the gedit experience and many of them are enabled by default.
Created attachment 278632 [details] [review] plugins: move zeitgeist to gedit-plugins project Ok. This patch should drop all zeitgeist-related code from 'gedit' project.
Created attachment 278633 [details] [review] plugins: add zeitgeist plugin from 'gedit' project This will add zeitgeist-related code to 'gedit-plugins' project.
Unfortunately, I've not tested these patches. I can test it at this weekned, so I think you could test it earlier.
It would be awesome if we could get the metainfo files upstream before 3.13.3 (three days time!) and then we can get some testing of the plugin installing and removal. Thanks!
Review of attachment 278632 [details] [review]: Seems good, please test the patch and if it is ok, feel free to push.
Review of attachment 278633 [details] [review]: In configure.ac, zeitgeist is placed in the Python plugins. But the plugin is written in C. Is there a reason to place it in the Python plugins? If so, a comment explaining why would be useful. ::: plugins/zeitgeist/Makefile.am @@ +23,3 @@ +plugin_in_files += plugins/zeitgeist/zeitgeist.plugin.desktop.in + +appstream_in_files += plugins/zeitgeist/gedit-zeitgeist.metainfo.xml.in See the Makefile.am for the git plugin, appstream_in_files is outside the if/else.
(In reply to comment #9) > Review of attachment 278633 [details] [review]: > > In configure.ac, zeitgeist is placed in the Python plugins. But the plugin is > written in C. Is there a reason to place it in the Python plugins? If so, a > comment explaining why would be useful. > > ::: plugins/zeitgeist/Makefile.am > @@ +23,3 @@ > +plugin_in_files += plugins/zeitgeist/zeitgeist.plugin.desktop.in > + > +appstream_in_files += plugins/zeitgeist/gedit-zeitgeist.metainfo.xml.in > > See the Makefile.am for the git plugin, appstream_in_files is outside the > if/else. that's my bug. we should use inside ifs. I will fix tonight.
Created attachment 278856 [details] [review] plugins: add zeitgeist plugin from 'gedit' project
Created attachment 278857 [details] [review] plugins: fix installing appstream files Reported-by: Sébastien Wilmet <swilmet@gnome.org> Reference: https://bugzilla.gnome.org/show_bug.cgi?id=731799 Signed-off-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Review of attachment 278632 [details] [review]: https://git.gnome.org/browse/gedit/commit/?id=3f640a84f67cba7088b94f468afa1a66a5b043d3
Review of attachment 278857 [details] [review]: Thanks, it's better.
Review of attachment 278856 [details] [review]: Only a small comment below, the rest looks good. Feel free to push after fixing the nitpick. ::: configure.ac @@ +176,3 @@ fi + + Nitpick: please remove those two newlines, it is not related to the patch, and newlines are not needed for ending a block. (And if a newline is better for readability, only one newline is enough.)
Created attachment 278859 [details] [review] plugins: add zeitgeist plugin from 'gedit' project found bug. fixed. now it works fine.
Review of attachment 278859 [details] [review]: https://git.gnome.org/browse/gedit-plugins/commit/?id=fc1cb38bea0245ade0a84a656b44a6f3da4b5fc5
Review of attachment 278857 [details] [review]: https://git.gnome.org/browse/gedit-plugins/commit/?id=db67b7822c16bf8a0224a27db1c843a5867770e5
Thank you for review! Let me know if I added bugs :D but I hope I didn't added.
As a general rule, once a patch is merged it's better to remove the associated wip/ branch, otherwise we end up with tons of useless branches.