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

Small Fixes and Tidy Up #45

Merged
merged 9 commits into from
Dec 17, 2024
Merged

Small Fixes and Tidy Up #45

merged 9 commits into from
Dec 17, 2024

Conversation

schuhmaj
Copy link
Collaborator

@schuhmaj schuhmaj commented Nov 28, 2024

Changelog

  • Fix actually utilized CMake Option not being properly declared as option
    • Hasn't been an issue so far because one could still set the right variable (also the docs were aligned to this), but no proper CMake
  • Fix an issue with missing fmt symbols using pre-installed spdlog on arm architecture
    • We're not using spdlog as header library instead of the maybe-present-pre-compiled library
  • Modernize Tetgen Integration to avoid CMake deprecation of legacy fetch content functions
  • Unify the versioning by adding a Info.h.in file and a corresponding version.cmake which fills in the values
  • Add meta attributes to python interface __version__, __parallelization__, __commit__ and __logging__ for the end-user to display compilation details

Using the correct option worked. However, it was not listed and declared as option.
Reorganize spdlog headers in CSVWriter.h and remove unused imports in other files to clean up code dependencies. Add a compile definition in CMakeLists.txt to resolve missing fmt symbols.
@schuhmaj schuhmaj self-assigned this Nov 28, 2024
@schuhmaj schuhmaj changed the title Small Fixes Small Fixes and Tidy Up Nov 29, 2024
@schuhmaj schuhmaj marked this pull request as ready for review December 9, 2024 12:49
@schuhmaj schuhmaj requested a review from gomezzz December 9, 2024 12:49
@schuhmaj schuhmaj added the enhancement New feature or request label Dec 9, 2024
Copy link
Collaborator

@gomezzz gomezzz left a comment

Choose a reason for hiding this comment

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

All good from my end but it seems the CI/CD doesn't like it yet :)

@schuhmaj schuhmaj force-pushed the fix-python-option branch 3 times, most recently from dcfc6ef to 0ce6c37 Compare December 13, 2024 12:26
Replaced hardcoded versioning with a dynamic CMake-driven system using `Info.h` to embed version, parallelization, and commit metadata directly into the build. Simplified Python version parsing and renamed `LOGGING_LEVEL` to `POLYHEDRAL_GRAVITY_LOGGING_LEVEL` for consistency. Consolidated version management and improved Git commit hash handling.
@schuhmaj schuhmaj requested a review from gomezzz December 13, 2024 12:36
@schuhmaj
Copy link
Collaborator Author

Thank you for the first review. I modernized the process a bit more. The variables are now filled in via CMake and the CI/CD is working.

Copy link
Collaborator

@gomezzz gomezzz left a comment

Choose a reason for hiding this comment

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

Looks good, I guess :D If there are specific things I should test to confirm it works lmk :)

@schuhmaj
Copy link
Collaborator Author

Thank you :) The pipeline should be sensitive to detect bugs if there were any 😇

@schuhmaj schuhmaj merged commit 78c0f2f into main Dec 17, 2024
6 checks passed
@schuhmaj schuhmaj deleted the fix-python-option branch December 17, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants