[Github-comments] [geany/geany-plugins] commander: use new plugin API. (#703)

[LarsGit223]

> And there's at least one unrelated change (help location), which should at least be a separate commit.

Ok, I did not realize that it is opening the local file. Will remove it from this PR.

> I fail to see what value these changes give, given that the "old" API is fully functional (we spent some effort to make sure it'd still work flawlessly), and the new one, while nicer, doesn't provide any actual new feature this plugin would need. This looks like a change for the sake of change, which I don't really fancy (one reason being a bug likeliness to improvement ration very low). Also, if this gets done, it should fully embrace the new API philosophy and try and get rid of the global variables (unless actually problematic).

Hmmm....well it is not a change for the user in terms of functionality of course. But IMHO it is not just a change for the sake of change but a change for consistency and cleanup. Also, there was some reason to design the new API or not? The new API seems to be more easily extendable. This does not matter right now but maybe in the future if a API change/extension is required. I thought it would be nice to have all plugins using the new API and throw away the code which is required to support the old one.

And maybe I should not say "new API" but current API as it was introduced in Geany 1.26 (November 15, 2015).

Anyway, I should maybe have opened a discussion before starting this one. So I say sorry for my "bold" PR flood. This is your plugin, if you rather like to keep it like it is then I accept it of course.

-- 
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/703#issuecomment-368295698
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20180225/6bbe94c5/attachment.html>