The Wayback Machine - https://web.archive.org/web/20200912142214/https://github.com/arduino-cmake/Arduino-CMake-NG/pull/99
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

Fixed #98 by removing camelCasing of library names #99

Open
wants to merge 1 commit into
base: master
from

Conversation

@marthinwurer
Copy link

marthinwurer commented Feb 6, 2020

CamelCasing of library names caused issues with using the built-in SD library. I removed it, and fixed up the case where the stepper library example did not match that in the wiki docs. I also added an example with the SD library linked to act as a regression test.

This change might break existing code that uses the CamelCasing feature.

Copy link
Member

MrPointer left a comment

Besides the specific comments I've left, it looks that this PR doesn't pass all required checks.
As such, I can't approve this merge yet.

@@ -1,3 +1,4 @@
set(CMAKE_TOOLCHAIN_FILE ../cmake/Arduino-Toolchain.cmake)

This comment has been minimized.

@MrPointer

MrPointer Feb 12, 2020

Member

Unnecessary, please remove this line

@@ -27,10 +27,6 @@ function(find_arduino_library _target_name _library_name)

is_platform_library(${_library_name} is_plib) # Detect whether library is a platform library

if (NOT parsed_args_3RD_PARTY AND NOT is_plib)

This comment has been minimized.

@MrPointer

MrPointer Feb 12, 2020

Member

I'm afraid you can't simply delete this block.
It's been a long time since I've written any code in this project, so I don't exactly remember the details, but I do remember that this is important, for other libraries.
You've got to find another way around this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.