On 22/04/2012 20:20, Chow Loong Jin wrote:
On 23/04/2012 02:14, Chow Loong Jin wrote:
Could you include that explanation into the commit message of that patch, please?
Done.
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.
No, removed.
- "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)
That was just a question about dist it or not.
- What's up with the FIXME in geanyvc/src/Makefile.am? I don't think "…" as a FIXME message is particularly descriptive.
It has its own commit now (5/9).
- The following hunk is really unnecessary. I personally prefer spaces between # and the actual comment.
Reflex…
And finally... please separate the patch a little. Specifically, I think the unittest-related changes should go into their own commit.
No problem, planed and done now!
One more issue:
hyperair@thinkpwn:~/src/geany-plugins/multiterm/src [master]% make V=1 distdir
[ 2:17AM]rm -f multiterm_la_vala.stamp && echo stamp > multiterm_la_vala.stamp-t CDPATH="${ZSH_VERSION+.}:" && cd . && --vapidir ../../multiterm/src/vapi --pkg gtk+-2.0 --pkg geany --pkg vte --header multiterm.h --use-header -C config.vala context-menu.vala defconf.vala notebook.vala plugin.vala shell-config.vala tab-label.vala terminal.vala /bin/bash: --vapidir: command not found
You have to set VALAC when using Vala, and you can’t dist without Vala, it’s not a real issue but it’s still annoying.
Attached the patch suite (9 patches). The last one may be removed if it’s not yet desirable.
Cheers