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

ladislas/feature/cmake link libraries #270

Merged
merged 3 commits into from
Jun 3, 2021

Conversation

ladislas
Copy link
Member

@ladislas ladislas commented Jun 3, 2021

  • 🚚 (cmake): Rename remove lib_ prefix from lib target name
  • πŸ—οΈ (cmake): Link general purpose libraries to all targets
  • πŸ”₯ (cmake): Remove redundant linked libraries

@ladislas ladislas force-pushed the ladislas/feature/cmake-link_libraries branch from 97b039d to 80dc2dd Compare June 3, 2021 13:46
@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #270 (0799b9e) into develop (6fb068a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop      #270   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           56        56           
  Lines          936       936           
=========================================
  Hits           936       936           

Continue to review full report at Codecov.

Legend - Click here to learn more
Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data
Powered by Codecov. Last update 6fb068a...0799b9e. Read the comment docs.

@ladislas ladislas force-pushed the ladislas/feature/cmake-link_libraries branch from 80dc2dd to 26d8538 Compare June 3, 2021 14:08
spikes/lk_wifi/CMakeLists.txt Outdated Show resolved Hide resolved
link_libraries(
Utils
LogKit
HelloWorld
Copy link
Member

@YannLocatelli YannLocatelli Jun 3, 2021

Choose a reason for hiding this comment

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

Je ne sais pas si HelloWorld est pertinent. Les autres sont explicites si on veut les utiliser (LogKit pour les logs, Utils pour des calculs, ...) mais HelloWorld est vague dans son utilitΓ© parce qu'il n'a pas Γ©voluΓ© depuis le premier jour.

Aujourd'hui il n'affiche aucun message et fait clignoter la LED1 toutes les secondes.

Si il faut l'ajouter en terme de "bon fonctionnement" de la carte, il faudrait

  • soit modifier le nom comme BlinkyLed,
  • soit ajouter d'autres choses que la led, comme un log_info("A message from your board %s --> \"%s\" at %i s\n", MBED_CONF_APP_TARGET_NAME, hello.world, int(t.count() / 1000)); toutes les secondes

This change allows all targets to use the aforementioned libraries by
just including their header.

There is not need to use target_link_libraries anymore.
@ladislas ladislas force-pushed the ladislas/feature/cmake-link_libraries branch from 26d8538 to a1a3c38 Compare June 3, 2021 14:21
@ladislas ladislas force-pushed the ladislas/feature/cmake-link_libraries branch from a1a3c38 to 0799b9e Compare June 3, 2021 14:26
@codeclimate
Copy link

codeclimate bot commented Jun 3, 2021

Code Climate has analyzed commit 0799b9e and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Jun 3, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ladislas ladislas merged commit b5e5af4 into develop Jun 3, 2021
@ladislas ladislas deleted the ladislas/feature/cmake-link_libraries branch June 3, 2021 15:20
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.

2 participants