Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to cmake and mumble-plugin-cpp, also OO rewrite #82

Open
hbeni opened this issue Feb 16, 2021 · 6 comments · May be fixed by #84
Open

Switch to cmake and mumble-plugin-cpp, also OO rewrite #82

hbeni opened this issue Feb 16, 2021 · 6 comments · May be fixed by #84
Assignees
Labels
low prio Low priority mumble-plugin Affecting mumble plugin

Comments

@hbeni
Copy link
Owner

hbeni commented Feb 16, 2021

Robert adviced us:

I once again advise you to rework your plugin so it uses https://github.com/mumble-voip/mumble-plugin-cpp as a base. When using the cpp based API, memory management is automated and you never have to worry about it.

Additionally: We should refactor to use more OO structures to get rid of all the global variables and functions (ref #58)

@hbeni hbeni added mumble-plugin Affecting mumble plugin low prio Low priority labels Feb 16, 2021
@hbeni hbeni added this to the Soon™ (=Future) milestone Feb 16, 2021
@mill-j
Copy link
Collaborator

mill-j commented Feb 16, 2021

@hbeni I'm willing to invest some time on figuring this API out and maybe rewriting the plugin. From the little I've seen it looks a little more like what I'm used to working with. And I have a feeling it should be easier to maintain.

I haven't used cmake on any of my projects yet, but if we do switch, let's make sure we can parallel build ;) Currently make -j4 fails. Not a big deal on such a small project though.

So what do you say we keep the current codebase patched and in working order and work on the new one on the side till it's a seamless drop-in replacement?

@hbeni
Copy link
Owner Author

hbeni commented Feb 16, 2021

So what do you say we keep the current codebase patched and in working order and work on the new one on the side till it's a seamless drop-in replacement?

Good idea!
regarding cmake; i'm not sure if the switch is really needed for using the cpp-plugin interface. It would be good, however i have no idea of cmake yet.

@mill-j mill-j self-assigned this Feb 16, 2021
@Krzmbrzl
Copy link

regarding cmake; i'm not sure if the switch is really needed for using the cpp-plugin interface. It would be good, however i have no idea of cmake yet.

Strictly speaking it is not a requirement for using the wrapper. However all its dependencies and all the internal macro-management is done via cmake. So if you chose to use it in a different way, then that would
a) be unsupported by me
b) be a bit of work to get all the functionality cmake currently does into whatever other system you are using.

@hbeni
Copy link
Owner Author

hbeni commented Feb 17, 2021

@mill-j It would also be cool to convert the radio code and other modules to objectoriented, btw.

@mill-j
Copy link
Collaborator

mill-j commented Feb 17, 2021

Certainly, that should be easier than the current structs. Not to mention much easier to document with doxygen. What do you want to call the new branch?

@hbeni
Copy link
Owner Author

hbeni commented Feb 17, 2021

Pick your name - I usually do include the issue number tough.
Maybe Issue-82_OORwrite. Whatever you choose.

@hbeni hbeni linked a pull request Mar 8, 2021 that will close this issue
11 tasks
This was referenced Apr 20, 2021
@hbeni hbeni mentioned this issue Aug 30, 2021
5 tasks
@hbeni hbeni changed the title Switch to cmake and mumble-plugin-cpp Switch to cmake and mumble-plugin-cpp, also OO rewrite Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low prio Low priority mumble-plugin Affecting mumble plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants