-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Some small clean ups. #10416
Some small clean ups. #10416
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,6 @@ use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_I | |
use crate::util::config::{self, Config, SslVersionConfig, SslVersionConfigRange}; | ||
use crate::util::errors::CargoResult; | ||
use crate::util::important_paths::find_root_manifest_for_wd; | ||
use crate::util::validate_package_name; | ||
use crate::util::IntoUrl; | ||
use crate::{drop_print, drop_println, version}; | ||
|
||
|
@@ -344,7 +343,6 @@ pub fn registry_configuration( | |
// `registry.default` is handled in command-line parsing. | ||
let (index, token, process) = match registry { | ||
Some(registry) => { | ||
validate_package_name(registry, "registry name", "")?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this intentional to be removed? (in isolation I don't know what this is doing and it may no longer be necessary but otherwise it seems out of place for a cleanup-style commit) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I tryed to call it out in the PR:
So the next line calls this function. |
||
let index = Some(config.get_registry_index(registry)?.to_string()); | ||
let token_key = format!("registries.{}.token", registry); | ||
let token = config.get_string(&token_key)?.map(|p| p.val); | ||
|
@@ -418,8 +416,8 @@ fn registry( | |
} | ||
// Parse all configuration options | ||
let reg_cfg = registry_configuration(config, registry.as_deref())?; | ||
let opt_index = reg_cfg.index.as_ref().or_else(|| index.as_ref()); | ||
let sid = get_source_id(config, opt_index, registry.as_ref())?; | ||
let opt_index = reg_cfg.index.as_deref().or_else(|| index.as_deref()); | ||
let sid = get_source_id(config, opt_index, registry.as_deref())?; | ||
if !sid.is_remote_registry() { | ||
bail!( | ||
"{} does not support API commands.\n\ | ||
|
@@ -892,11 +890,7 @@ pub fn yank( | |
/// | ||
/// The `index` and `reg` values are from the command-line or config settings. | ||
/// If both are None, returns the source for crates.io. | ||
fn get_source_id( | ||
config: &Config, | ||
index: Option<&String>, | ||
reg: Option<&String>, | ||
) -> CargoResult<SourceId> { | ||
fn get_source_id(config: &Config, index: Option<&str>, reg: Option<&str>) -> CargoResult<SourceId> { | ||
match (reg, index) { | ||
(Some(r), _) => SourceId::alt_registry(config, r), | ||
(_, Some(i)) => SourceId::for_registry(&i.into_url()?), | ||
|
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.
Could this perhaps be
HashMap::from([...])
to avoid the need to have type ascription?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 will switch (them all) to that in the morning. It is much clearer.