Skip to content

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

Merged
christianlangevin merged 1 commit into
modflowpy:developfrom
wpbonelli:resolve-exe
Sep 24, 2023
Merged

fix(resolve_exe): support extensionless abs/rel paths on windows#1957
christianlangevin merged 1 commit into
modflowpy:developfrom
wpbonelli:resolve-exe

Conversation

@wpbonelli
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

@christianlangevin christianlangevin 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.

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