-
Notifications
You must be signed in to change notification settings - Fork 4
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
Enable allow-prereleases on all Python installs. #172
Open
freakboy3742
wants to merge
8
commits into
main
Choose a base branch
from
auto-dev-python
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
c477aa6
Enable allow-prereleases on all Python installs.
freakboy3742 9ae5062
Fall back to default Python for most tests.
freakboy3742 09f788e
Allow 'system' as a value for python-version.
freakboy3742 d142ed4
Correct quotation usage.
freakboy3742 3855e3f
Actually use the system setting value.
freakboy3742 6afc2d9
Simplify handling of system python version.
freakboy3742 afec009
Tweak handling of system to avoid need for sudo.
freakboy3742 464871d
Correct step output lookup.
freakboy3742 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Few thoughts reviewing this workflow:
app-build-verify.yml
explicitly specifiespython-version
; this seems unnecessary and should probably just use the default3.x
app-build-verify.yml
to allow the valuesystem
forpython-version
; that would signalapp-build-verify.yml
to skipactions/setup-python
without the caller needing to explicitly specify the version of Python for the runner.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 must be looking at something else, because to my eye, all the tests of app-build-verify (in ci.yml) explicitly set a python version:
Agreed this would be a useful feature to add.
To "yes-and" this - should we replace the existing system-python handling? The current handling has a bunch of "if the selected python version is the system python, don't install/do run this step" etc; should we replace that with "if the selected python version is
system
", and if the user happens to select a version of Python that happens to be the system python, then they get a "normal" python install?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.
Right; I'm just saying I think we should not set
python-version
inci.yml
and instead just let it fallback to the default (i.e.3.x
) specified inapp-build-verify.yml
.Yeah; this has all grown organically a bit and these simplifications definitely make sense to me.
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.
Ah - agreed, except for keeping one as a token check of the "explicitly asking for a python version" case.
Ok - I've made these two changes - let me know what you think.