-
Notifications
You must be signed in to change notification settings - Fork 406
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
Support additional registries #942
Support additional registries #942
Conversation
/cc @wt - sorry this took longer to get out than I expected, there was a lot of rebasing to do! |
Don't be sorry. And keep being awesome. |
86b3d19
to
13ee51e
Compare
13ee51e
to
8108cf8
Compare
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.
Can you explain what this change is doing? I think I misunderstood it initially. Is this adding plumbing to add a Cargo config to the cargo metadata
call?
Also one nit 😄
@@ -6,9 +6,9 @@ load("//rust:defs.bzl", "rust_common") | |||
load("//rust:repositories.bzl", "DEFAULT_TOOLCHAIN_TRIPLES") | |||
load("//rust/platform:triple_mappings.bzl", "system_to_binary_ext", "triple_to_system") | |||
|
|||
DEFAULT_CRATE_REGISTRY_TEMPLATE = "https://crates.io/api/v1/crates/{crate}/{version}/download" | |||
DEFAULT_DEFAULT_CRATE_DOWNLOAD_URL_TEMPLATE = "https://crates.io/api/v1/crates/{crate}/{version}/download" |
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.
DEFAULT_DEFAULT
?
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.
Intentional (but definitely looks weird!)
DEFAULT_CRATE_DOWNLOAD_URL_TEMPLATE
is the crate download URL template for crates from the default registry (i.e. crates.io)
This is the default value for that
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.
Maybe add a comment above this then?
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.
Fair - added comment :)
Yes - this is the interface |
I see, I think this looks fine then, pending my other request. Nice job with the paired change! |
No description provided.