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

Support alternate virtualenv directories #11

Merged
merged 1 commit into from
Jun 7, 2022
Merged

Support alternate virtualenv directories #11

merged 1 commit into from
Jun 7, 2022

Conversation

Exagone313
Copy link
Contributor

@Exagone313 Exagone313 commented May 9, 2022 •
edited
Loading

Fix #10

EDIT: This could be replaced by a custom environment variable, such as:

PYLINT_VENV_DIRECTORIES=.venv:.pyenv:.virtualenv

@Exagone313 Exagone313 changed the title Support alternate virtualenv directories #10 Support alternate virtualenv directories May 9, 2022
jgosmann
jgosmann requested changes May 17, 2022
View reviewed changes
Copy link
Owner

@jgosmann jgosmann left a comment

Choose a reason for hiding this comment

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

Thank you! In general this looks good to me. Could you also update README.rst (it states the folder names) and prepare an entry in CHANGES.md?

@Exagone313
Copy link
Contributor Author

@jgosmann, are you good for the code I made or do you think it would be better to add environment variable support like I suggested?

@jgosmann
Copy link
Owner

I'm fine with either. I think the current version is already an improvement, having an environment variable would add some more flexibility on top.

@Exagone313 Exagone313 marked this pull request as draft June 5, 2022 16:57
@Exagone313
Support alternative virtualenv directories jgosmann#10
f426644 
This introduces a new PYLINT_VENV_PATH environment variable to pass a
list of paths that will be checked.
@Exagone313
Copy link
Contributor Author

Exagone313 commented Jun 5, 2022 •
edited
Loading

@jgosmann, I updated it. I named the environment variable PYLINT_VENV_PATH. I also updated the files you mentioned.

Unrelated to this, I noticed that Pylint 2.14.0 (released a few days ago) broke multiline init-hook, so it's no longer possible to run the try-except (as far as I know it can't be made on a single line). EDIT: unsure about this, I couldn't reproduce it anymore.

@Exagone313 Exagone313 marked this pull request as ready for review June 5, 2022 19:08
jgosmann
jgosmann reviewed Jun 7, 2022
View reviewed changes
pylint_venv.py
# Check if a local ".venv" folder exists
# Check if a virtualenv directory exists (.venv by default, or alternative
# paths given by environment variable)
venv_paths = os.getenv("PYLINT_VENV_PATH", ".venv").split(":")
Copy link
Owner

Choose a reason for hiding this comment

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

Note: This does not support any directories where the name itself contains a colon (:). However, I think it is fine because the same is true for Bash's $PATH variable and I hope no-one uses colons in the venv directory names (or any other directory names for that matter).

@jgosmann jgosmann merged commit 0634188 into jgosmann:master Jun 7, 2022
@jgosmann
Copy link
Owner

jgosmann commented Jun 7, 2022

Unrelated to this, I noticed that Pylint 2.14.0 (released a few days ago) broke multiline init-hook, so it's no longer possible to run the try-except (as far as I know it can't be made on a single line).

I was able to reproduce this and reported the issue upstream as pylint-dev/pylint#6888.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgosmann jgosmann jgosmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for more locations
2 participants
@Exagone313 @jgosmann