Nothing more to say. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/708
-- Commit Summary --
* geanyctags: use new plugin API
-- File Changes --
M geanyctags/src/geanyctags.c (56)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/708.patch https://github.com/geany/geany-plugins/pull/708.diff
techee approved this pull request.
Sorry, completely forgot about this one. LGTM.
Maybe one question - what Geany API does this change require? Is the currently used API version specified in the plugin enough?
Also is the previous API deprecated or something? The patch is kind of "why not" but I slightly fail to see "why yes" reasons.
Also is the previous API deprecated or something? The patch is kind of "why not" but I slightly fail to see "why yes" reasons.
Please also see the discussion in #703. It's purely for consistency/cleanup. I wonder if there is a new API why not move to it. Otherwise the old one will/can never be deprecated in the future. Anyway - as written in the discussion I would of course accept if you do not like to merge it.
Maybe one question - what Geany API does this change require? Is the currently used API version specified in the plugin enough?
According to https://www.geany.org/manual/reference/legacy.html it's deprecated since Geany 1.26, don't know the API version. But it is NOT obsolete and there is NO plan to remove it. So it will just work fine.
So as writen above I will accept a "no" without any discussion and the change is only because I think it's nicer to not use a deprecated API even if support for it will be ongoing.
According to https://www.geany.org/manual/reference/legacy.html it's deprecated since Geany 1.26, don't know the API version.
I was asking because if the API version used by the plugin was older, you'd have to bump the API too. But I've just checked and 226 which the plugin uses corresponds to the 1.26 release so it's OK for this plugin. But you should check other plugins too.
As I said, the patch looks good to me and I have no problem with it - I was just wondering about the reason and you gave me the answer.
I was asking because if the API version used by the plugin was older, you'd have to bump the API too.
True, thanks for the reminder. The PRs are a bit older, I don't remember if I checked that or not. I will do a re-check some time later.
@frlan: according to @techee this PR looks fine so it could be merged. Unless I misunderstood something, there are no pending fixes. The travis CI build failed because of the webhelper plugin and that is not related to my changes.
Merged #708 into master.
github-comments@lists.geany.org