@codebrainz Please check - pretty trivial though so probably correct. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/438
-- Commit Summary --
* multiterm: Drop deprecated geany_functions and bump API requirement
-- File Changes --
M multiterm/src/plugin.vala (3)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/438.patch https://github.com/geany/geany-plugins/pull/438.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438
Seems kind of pointless to bump the required API just to remove a harmless line of code. If you're adamant, I don't really mind though, as long as the plugin still builds and works.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438#issuecomment-225059815
The geany_functions declaration was required before the introduction of geany shared library, which corresponds to API 224. When geany_functions gets removed, the plugin won't work on older Geany versions anyway so better to announce it using the API. I did this change in the past for other plugins too, see https://github.com/geany/geany-plugins/pull/388 and the linked bug.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438#issuecomment-225111091
Yeah, I know what it is (I wrote the original commit removing it), I just mean since it's not removed from Geany, why make the plugin artificially require a way newer Geany version, just so one line of harmless code can be removed?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438#issuecomment-225201367
Because there are way too many deprecation warnings when compiling GP and it's easy to overlook the important ones (happened to me before). Now it's true multiterm generates quite many warnings anyway because of the vala-generated code (any way to suppress that?) so one extra doesn't matter that much.
I'll leave the decision up to you - IMO not many people use new plugins with old Geany version so the change wouldn't hurt anyone.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438#issuecomment-225212670
Couldn't we just not use a compiler builtin to nag about deprecations in Geany? It's pretty rare that we ever use these, usually just the API documentation has `@deprecated` added. It just seems weird to churn so many plugins, bumping their API dependency, just to avoid warnings we ourselves cause (annoying, I agree).
Anyway, I don't really care that much. As long as the plugin still builds and functions.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/438#issuecomment-225317701
So merging yes/no? Let's start the poll ;)
github-comments@lists.geany.org