-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: Detect Vyper version from source #23
feat: Detect Vyper version from source #23
Conversation
acc5243
to
1571879
Compare
detect_vyper_version_from_source
vvm/main.py
Outdated
--------- | ||
specifier : SpecifierSet | ||
Specifier to pick a version for. | ||
prereleases : bool, optional |
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.
either default to false (kosher, since False
is not a mutable in python), or add a line in the function body like if prereleases is None: prereleases = False
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.
the packaging
lib will only choose prereleases when they are the only ones available
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.
i'm more talking about the default value -- no need to pick between None, True and False
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.
I think None is the correct value here
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, left some small style comments
Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
What I did
How I did it
How to verify it
Checklist