← Back to team overview

lubuntu-desktop team mailing list archive

Re: Call for review: PCManFM is almost ready for a new release.

 

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


Follow ups

References