-
Notifications
You must be signed in to change notification settings - Fork 71
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
Refactor tool downloading and output, add Windows MiniForge example #98
Refactor tool downloading and output, add Windows MiniForge example #98
Conversation
Very close now... |
@@ -39,7 +39,7 @@ jobs: | |||
- uses: actions/checkout@v2 | |||
- uses: ./ | |||
with: | |||
installer-url: https://github.com/conda-forge/miniforge/releases/download/4.8.3-2/Miniforge-pypy3-4.8.3-2-MacOSX-x86_64.sh | |||
installer-url: https://github.com/conda-forge/miniforge/releases/download/4.9.0-3/Miniforge-pypy3-4.9.0-3-MacOSX-x86_64.sh?foo=bar&baz |
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.
?foo=bar&baz
Is this related to cache?
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.
Oh nevermind, you used it to test the url parsing.
Right @bollwyvl ?
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.
Yep, just want to make sure we've got an out for the inevitable s3 trash or wherever people are putting these things. Actually, going to add some docs about that...
|
||
## Fixes | ||
|
||
- [#97][] fixes `installer-url` on Windows |
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.
Hmm good point should have added this to the other PRs :-p
@jaimergp would you like to update this on the changelog with the PRs you started?
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.
Oops, yep, will do!
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.
No worries I forgot too 🤦🏽
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.
Awesome work @bollwyvl
Thanks a lot!
Thanks for the review! Looking over the logs, |
Ah, I was banging my head against paths and escaping: turns out I still think we want to use the |
Agreed. Good to merge then? |
Sorry my attention is a bit async right now... it looks like No further changes planned! |
You pinky promise? |
we swears. I'm working conda-forge chores now 😫 |
Code Changes:
&?#