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

Consider Nightly channels when checking for avaliable_channels #848

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Consider Nightly channels when checking for avaliable_channels #848

wants to merge 3 commits into from

Conversation

christiangnrd
Copy link
Contributor

@christiangnrd christiangnrd commented Feb 26, 2024 •
edited
Loading

Closes #820 but not #995.

@christiangnrd christiangnrd force-pushed the invalid-nightly branch 2 times, most recently from d8181c0 to fd02621 Compare February 26, 2024 20:10
@christiangnrd christiangnrd mentioned this pull request Jul 16, 2024
Open
@christiangnrd christiangnrd force-pushed the invalid-nightly branch 2 times, most recently from 58cec41 to a5aa2db Compare July 16, 2024 18:03
@christiangnrd christiangnrd mentioned this pull request Jul 16, 2024
Open
@christiangnrd
Copy link
Contributor Author

This is ready for review.

@christiangnrd christiangnrd mentioned this pull request Jul 16, 2024
Draft
davidanthoff
davidanthoff requested changes Aug 21, 2024
View reviewed changes
Copy link
Collaborator

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

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

Nice!

I just have some change requests in how errors are handled. 1) we should use ? more, and 2) always bubble errors up, so that if something goes wrong that everything fails and we get an error report :)

src/operations.rs Outdated Show resolved Hide resolved
src/operations.rs Outdated Show resolved Hide resolved
src/operations.rs Outdated Show resolved Hide resolved
@christiangnrd
Copy link
Contributor Author

christiangnrd commented Aug 21, 2024 •
edited
Loading

Thanks for the review! I don't have much Rust experience so this is very helpful. I've addressed your comments along with the required fixes.

@christiangnrd christiangnrd force-pushed the invalid-nightly branch 2 times, most recently from 36fd1bd to dc94ba4 Compare September 4, 2024 12:43
@christiangnrd
Add is_valid_channel and use it everywhere `version_db.available_cha…
170d9d5 
…nnels` is used
@christiangnrd
Phrasing and comments
ce2347c 
"from" makes more sense to me as the error message seems to be attempting to convey where `julialauncher` got the missing/invalid channel from.
@christiangnrd
Address feedback
89841a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidanthoff davidanthoff Awaiting requested review from davidanthoff

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.

Error says: "Invalid Juliaup channel nightly"
2 participants
@christiangnrd @davidanthoff