On 23/04/2012 00:55, Quentin Glidic wrote: > On 22/04/2012 18:21, Chow Loong Jin wrote: >> On 22/04/2012 20:09, Quentin Glidic wrote: >>> Hello, >>> >>> I noticed a global misuse / misunderstanding of the Autotools >>> (mostly about the dist mechanism) among the geany-plugins. >> >> Could you elaborate, please? >> >> I noticed that you have shifted all the ENABLE_$PLUGIN conditionals >> to the top-level Makefile.am instead, by modifying the SUBDIRS >> variable there. However, this is not desirable as disabled plugins >> will be excluded from the release tarball generated by `make dist'. >> >> --enable/disable-$plugin should only toggle the building of the >> actual plugin. > > That’s exactly the misunderstanding I was speaking about. From the > Automake manual: > > “If SUBDIRS is defined conditionally using Automake conditionals, > Automake will define DIST_SUBDIRS automatically from the possible values > of SUBDIRS in all conditions.”
Great! I hadn't known of the existence of the DIST_SUBDIRS variable. I do recall running into issues where whole plugins went missing from the release tarball using that approach though. Could you include that explanation into the commit message of that patch, please? > [...] > Automake handles the conditionnal perfectly by itself, and just need > some "dist_" prefixes here and there, while EXTRA_DIST should be > reserved for files that are used outside of automake scope. Of course. I believe they were mostly used in the past for the AM_CONDITIONAL separation. Apart from that.. it looks mostly good, but here are a couple of questions/issues: - Is there a reason you defined plugin = geanydoc in geanydoc/src/Makefile.am? It doesn't look like it's needed there. - "FIXME: CSS?" doesn't look like it's needed in geanygendoc -- there's a rule to generate manual.html from manual.rst and manual.css. (This probably shouldn't be dist'd, but Colomban would probably be in a better position to answer that) - What's up with the FIXME in geanyvc/src/Makefile.am? I don't think "…" as a FIXME message is particularly descriptive. - The following hunk is really unnecessary. I personally prefer spaces between # and the actual comment. > diff --git a/pretty-printer/Makefile.am b/pretty-printer/Makefile.am > index 8c643f8..a82e819 100644 > --- a/pretty-printer/Makefile.am > +++ b/pretty-printer/Makefile.am > @@ -1,4 +1,4 @@ > -# include $(top_srcdir)/build/vars.auxfiles.mk > +#include $(top_srcdir)/build/vars.auxfiles.mk > > SUBDIRS = src > plugin = codenav And finally... please separate the patch a little. Specifically, I think the unittest-related changes should go into their own commit. -- Loong Jin
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Geany-devel mailing list [email protected] https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
