Skip to content

Make all plugins CamelCase - #1286

Closed
FloEdelmann wants to merge 5 commits into
OpenLightingProject:masterfrom
FloEdelmann:spi-refactor
Closed

Make all plugins CamelCase#1286
FloEdelmann wants to merge 5 commits into
OpenLightingProject:masterfrom
FloEdelmann:spi-refactor

Conversation

@FloEdelmann

Copy link
Copy Markdown
Member

As discussed in #1285, I refactored the SPI plugin to change the C++ name prefix to Spi. To standardize all plugins, I did the same for the GPIO, OPC and OSC plugins.

Unfortunately, I get the following errors when makeing, although I cannot find references to the old names in the code anymore:

olad/.libs/libolaserver.so: undefined reference to `vtable for ola::plugin::osc::OscPlugin'
olad/.libs/libolaserver.so: undefined reference to `vtable for ola::plugin::spi::SpiPlugin'
olad/.libs/libolaserver.so: undefined reference to `vtable for ola::plugin::gpio::GpioPlugin'
olad/.libs/libolaserver.so: undefined reference to `vtable for ola::plugin::openpixelcontrol::OpcPlugin'

Am I missing something or are these old build artefacts and I just have to rebuild completely (I have already run make clean)?

@peternewman peternewman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over to @nomis52 really. I'm beginning to wonder if it's not a good idea for backwards compatibility if people have done weird stuff and if we should just put up with it...

Also I'm rather confused by:
d245842

As it all seems to render fine for me in new and old UIs before you made this change.

@peternewman peternewman added this to the 0.11.0 milestone Jul 19, 2017
@FloEdelmann

Copy link
Copy Markdown
Member Author

d245842 actually makes it standard Markdown compliant which enabled me to preview it right in my editor which doesn't understand GFM. I'm rather confused why the line breaks worked before, since GFM does also require 2 spaces at the end of a line. It shouldn't break things though.

@peternewman peternewman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @FloEdelmann sorry for messing you about, but having had a ponder and talked to @nomis52 , our codebase capitalises acronyms, rather than camel casing them (e.g. https://github.com/OpenLightingProject/ola/blob/master/include/ola/network/TCPSocket.h and https://github.com/OpenLightingProject/ola/blob/master/include/ola/rdm/RDMAPI.h ) so would you mind reverting this please. If you're still feeling keen, it's actually UARTDMX, USBDMX, USBPro, FTDIDMX and OpenDMX that want updating.

Regarding GFM/MD and d245842 , can you do that in a separate PR for discussion, but my feeling is the link bit shouldn't be tweaked, I deliberately went for GFM so it renders best in GitHub along with the code (but can also be rendered fine in the UIs).

@FloEdelmann

Copy link
Copy Markdown
Member Author

Well, then UARTDMX, USBDMX, USBPro, FTDIDMX and OpenDMX can all be done in a separate pull request and this one can be closed. I'll raise an issue for that.

@FloEdelmann FloEdelmann closed this Sep 4, 2017
@FloEdelmann FloEdelmann mentioned this pull request Sep 4, 2017
5 tasks
@FloEdelmann
FloEdelmann deleted the spi-refactor branch September 4, 2017 14:44
@peternewman

Copy link
Copy Markdown
Member

Thanks again @FloEdelmann and sorry for the change of mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants