From: Jonathan M. <jma...@fa...> - 2011-06-19 13:27:51
|
On 06/18/2011 10:07 PM, PCMan wrote: >> Add LINGUAS, reference doc files and libfm-pref-apps.desktop launcher: > We do not translate desktop files directly. Neither do we commit the > translated desktop files. OK... then this patch is not needed upstream. I'm not sure why it was done this way as a patch in the Debian packaging, in that case. Perhaps a historical artifact? > We did the translation for desktop files in po files, and only put > *.desktop.in files in git. I think I see what could have happened here; the package build process was not running all the autoconf tools the way autogen.sh does, and so it tried to work around that. I have very recently created some changed packaging files in lp:~lxde/pcmanfm/libfm-packaging which *do* run all the needed autoconf and intltoolize stuff, and that seems to work without the translated files being in the tree, because it generates them at build time. >> Ensure correct icon is used in panel. > A better fix is to set the icon-name property for the window in > GtkBuilder glade file instead. OK. I am more of a command-line person than a GUI programmer, the GTK UI stuff is relatively new to me. As I recall, I fixed the bug in the way Julien suggested it be fixed. There were several similar issues with icon names not being set in various LXDE-related tools. I fixed them all in a similar way to this, and those patches were accepted by Julien into the Ubuntu packaging. I can redo this in glade if that is the correct approach. I'm not really sure I understand the benefit of moving one line from code into a glade file, though. >> Add GLIB_LIBS when linking documentation. > This seems to be fine, but I'm not sure which files should be pushed > to git repo since some are generated files. This commit should have added exactly one line to exactly one file: docs/reference/libfm/Makefile.am The debian/patches/04_fix_docs_linker.patch file it was based on only affects that one line. If the commit did more than that, that was a mistake I made! As far as I can tell, it only changed that one line, see https://gitorious.org/lxde-jmarsden/libfm-jmarsden/commit/284a1640d146b7bc6483ebb0b02058a78e914444 I *think* you should have been able to git cherrypick this commit directly into your master git repository, where it should have changed just that one file; that was my intention, at least. If that didn't "work", then we should probably try to figure out why, so that we can do this kind of cooperation more smoothly next time. >> Disable deprecated gio code by default. > This looks fine, but will this affect distros with older versions of glib? Possibly, I'm not sure. However, are new releases of pcmanfm intended for backporting to those older distros? I'm not sure this is an issue in practice for a new pcmanfm/libfm release, unless current development releases of major distros (Fedora, SuSE, Debian, ...) are still using older glib versions. > Specify default terminal emulator (was: 01-lxde-conf.patch in Ubuntu > The x-terminal-emulator thing IIRC is Debian-specific. So this better > goes to debian package rather than upstream. OK, then yes, that should stay as a patch, in Debian/Ubuntu packaging. I'd definitely like to see the GLIB_LIBS change included upstream, as (I think) it will allow me to re-enable gtk-doc use in my test packages made from the libfm git sources. Removing the deprecated gio code would also be good to see, too, unless it will cause issues building libfm on other current distributions. I'll take a look at the glade change idea for the icon, but not this weekend. Jonathan |