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

fix(resolve_exe): support extensionless abs/rel paths on windows #1957

Merged
merged 1 commit into from
Sep 24, 2023

Conversation

wpbonelli
Copy link
Member

The motivation here is symmetry of portability — relative paths with .exe suffix will work on non-Windows, and paths without .exe suffix will work on Windows, provided the relpath is otherwise identical

@wpbonelli wpbonelli requested a review from mbakker7 September 21, 2023 13:20
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #1957 (b24e38d) into develop (9e9d730) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           develop   #1957   +/-   ##
=======================================
  Coverage     72.6%   72.6%           
=======================================
  Files          257     257           
  Lines        57812   57818    +6     
=======================================
+ Hits         42009   42029   +20     
+ Misses       15803   15789   -14     
Files Changed Coverage Δ
flopy/mbase.py 70.0% <100.0%> (+0.7%) ⬆️

... and 3 files with indirect coverage changes

@wpbonelli wpbonelli marked this pull request as ready for review September 21, 2023 13:47
@wpbonelli
Copy link
Member Author

after reconsidering the comment here

could easily be added for the next minor version release

IMO this is more of a fix than a feature so we might wrap it into 3.4.3 (planned for this weekend)

@wpbonelli
Copy link
Member Author

@langevin-usgs it would be good to get this into 3.4.3. CI will fail on develop after merging, but it shouldn't affect the release as bugfix commits will be cherry-picked

Copy link
Contributor

@langevin-usgs langevin-usgs left a comment

Choose a reason for hiding this comment

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

Maybe this is end of this issue? Thanks, @wpbonelli.

@langevin-usgs langevin-usgs merged commit d7d7f66 into modflowpy:develop Sep 24, 2023
21 checks passed
@wpbonelli wpbonelli deleted the resolve-exe branch September 24, 2023 18:10
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.

bug: location of mf6 executable using relative path
2 participants