-
Notifications
You must be signed in to change notification settings - Fork 9
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
Use portable shebangs #147
Conversation
@@ -1,4 +1,5 @@ | |||
#!/usr/bin/env python | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the noise. I made a global search and replace for shebangs and even the "good ones" got an extra line in the change.
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one will always call on the system Python causing some grief to users trying to use vens
or conda
.
@@ -1,3 +1,4 @@ | |||
#! /home/ubuntu/miniconda/envs/wofpy/bin/python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is trying to call a miniconda
installation from a user named ubuntu
in an env named wofpy
.
The changes another user besides @lsetiawan to have that configuration is quite slim 😄
PS: we really need to focus on more tests to catch these things early!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ocefpaf My bad... forgot to change those..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not your fault. I have my laptop setup to do the same. The only way to "defend" us against this is to add more tests. We need at least a full integration tests that tries to run an instance of wofpy
in Travis-CI.
Thanks, @ocefpaf. I won't touch this; @lsetiawan can review it tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for all the changes @ocefpaf, now I know the proper shebang! 😃
May help with #73 (comment)