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

Add install_dir option #268

Conversation

SeanBryan51
Copy link
Collaborator

@SeanBryan51 SeanBryan51 commented Mar 18, 2024

Currently benchcab assumes the compiled executables are installed under the src/offline directory, however this may not always be the case (e.g. this script installs binaries under <project_root>/bin). This change adds the install_dir option which lets the user specify the directory in which benchcab can find the built executables.

Fixes #259

@SeanBryan51 SeanBryan51 linked an issue Mar 18, 2024 that may be closed by this pull request
Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 63.95%. Comparing base (ac2cb1f) to head (51591e0).
Report is 7 commits behind head on main.

Files Patch % Lines
benchcab/model.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #268      +/-   ##
==========================================
+ Coverage   63.84%   63.95%   +0.10%     
==========================================
  Files          35       35              
  Lines        2597     2616      +19     
==========================================
+ Hits         1658     1673      +15     
- Misses        939      943       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SeanBryan51 SeanBryan51 force-pushed the 259-option-for-specifying-the-location-of-installed-executables branch from d6d6eb5 to dc5c0a7 Compare March 19, 2024 01:13
@SeanBryan51 SeanBryan51 marked this pull request as ready for review March 19, 2024 01:44
@SeanBryan51 SeanBryan51 force-pushed the 259-option-for-specifying-the-location-of-installed-executables branch from dc5c0a7 to 0afa73f Compare March 20, 2024 04:26
@SeanBryan51 SeanBryan51 force-pushed the 259-option-for-specifying-the-location-of-installed-executables branch from 0afa73f to 51591e0 Compare March 21, 2024 04:21
@SeanBryan51 SeanBryan51 requested review from a team and bschroeter and removed request for a team March 21, 2024 04:49
@SeanBryan51
Copy link
Collaborator Author

Integration tests are passing. I've also tested the new functionality with a version of CABLE built using CMake which requires setting install_dir to the bin directory.

@SeanBryan51 SeanBryan51 added priority:high High priority issues that should be included in the next release. and removed priority:high High priority issues that should be included in the next release. labels Mar 21, 2024
@SeanBryan51 SeanBryan51 merged commit f19781b into main Mar 21, 2024
4 checks passed
@SeanBryan51 SeanBryan51 deleted the 259-option-for-specifying-the-location-of-installed-executables branch March 21, 2024 23:47
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.

Option for specifying the location of installed executables
2 participants