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.

Sign up for GitHub

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

Jump to bottom

Use platform specific command to find path #1143

Merged
merged 5 commits into from
Dec 10, 2021
Merged

Use platform specific command to find path #1143

merged 5 commits into from
Dec 10, 2021

Conversation

ThomasWilshaw
Copy link
Contributor

@ThomasWilshaw ThomasWilshaw commented Nov 13, 2021 •
edited
Loading

Summarize your change.

Tests on Windows were failing as the test code uses the Unix command which. Here we change that to use a platform specific command (where) when running on Windows. shutil.which() cannot be used as it's Python3.3+ only.

I should also say I don't have a Linux machine to test on so this has only been tested on Windows.

ssteinbach
ssteinbach approved these changes Nov 13, 2021
View reviewed changes
Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

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

considered doing this another way, but I think this is fine, actually. Thanks!

@meshula
Copy link
Collaborator

meshula commented Dec 10, 2021

linux failing with

./tests/test_console.py:32:1: F401 'fnmatch' imported but unused
15
./tests/test_console.py:87:1: W293 blank line contains whitespace
16
make: *** [Makefile:146: lint] Error 1
17
Error: Process completed with exit code 2.

any thoughts?

@ThomasWilshaw
Copy link
Contributor Author

Sorry I'd left that in from a previous test. I've removed it in the latest commit.

@meshula
Copy link
Collaborator

meshula commented Dec 10, 2021

Thanks!

@meshula meshula merged commit 6be5d24 into AcademySoftwareFoundation:main Dec 10, 2021
jminor pushed a commit that referenced this pull request Feb 11, 2022
@ThomasWilshaw @jminor
Use platform specific command to find path (#1143)
3966b31 
* Use platform specific command to find path

* Remove usage of which as it is being deprecated

* Update based on PR comments.

* Cleanup

* Remove blank line
@ssteinbach ssteinbach added this to the Public Beta 15 milestone Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeanChristopheMorinPerso JeanChristopheMorinPerso JeanChristopheMorinPerso left review comments

@jfpanisset jfpanisset jfpanisset left review comments

@meshula meshula meshula approved these changes

@ssteinbach ssteinbach ssteinbach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Public Beta 15
Development

Successfully merging this pull request may close these issues.
5 participants
@ThomasWilshaw @meshula @ssteinbach @jfpanisset @JeanChristopheMorinPerso