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

BSK-185: Fixes case issue for builds with OpenCV on Linux #189

Merged

Conversation

scout-andrew
Copy link
Contributor

Description

This implements the fix described in #185 .

Verification

After making the fix, Basilisk was built using
~/basilisk $python conanfile.py --opNav True
and tests were run using
~/basilisk/src $ python -m pytest .
Tests passed.

Documentation

Unknown and likely 0

Future work

This could be caught by making sure all build options are run through CI/CD on all platforms. Alternatively, BSK's codebase should check for case sensitivity when dealing with file system operations, such as loading data or during builds.

@schaubh schaubh self-requested a review February 16, 2023 19:15
@schaubh
Copy link
Contributor

schaubh commented Feb 16, 2023

@scout-andrew , I need you to sign a non-CU BSK contributor agreement to contribute code to BSK. Could you please email me so I can send you the PDF to fill out? We plan to integrate this process into GitHub, but for now it is still manual through me.

Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

All worked on my system and everything build without issue and all tests passed. Please add a quick note in bskReleaseNotes.rst on this fix. I provided a suggested statement.

src/cmake/usingOpenCV.cmake Show resolved Hide resolved
@scout-andrew scout-andrew force-pushed the hotfix/bsk-185-linux-opnav-builds branch from 4a84001 to 7d989d3 Compare February 23, 2023 23:16
@patkenneally patkenneally merged commit 9b731f3 into AVSLab:develop Feb 24, 2023
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