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.

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

feat: add strict channel priority option #385

Merged
merged 8 commits into from
Oct 21, 2023

Conversation

ruben-arts
Copy link
Collaborator

@ruben-arts ruben-arts commented Oct 19, 2023

Skips options for a package in other channels if it is already found in channels that are defined first.

Comment on lines 143 to 149
if strict_channel_priority {
if let Some(url) = found_in_channel.clone() {
if repo_data.channel.base_url != url {
continue;
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be done much shorter:

Suggested change
if strict_channel_priority {
if let Some(url) = found_in_channel.clone() {
if repo_data.channel.base_url != url {
continue;
}
}
}
if found_in_channel.map_or(false, |c| c != &repo_data.channel.base_url) {
continue;
}

You dont need to check the strict_channel_priority because the found_in_channel is only set if that is true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is much better!

Comment on lines 176 to 178
if strict_channel_priority && !records.is_empty() {
found_in_channel = Some(repo_data.channel.base_url.clone());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just store a reference to forgo a clone:

Suggested change
if strict_channel_priority && !records.is_empty() {
found_in_channel = Some(repo_data.channel.base_url.clone());
}
if strict_channel_priority && !records.is_empty() {
found_in_channel = Some(&repo_data.channel.base_url);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice!

crates/rattler_repodata_gateway/src/sparse/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

LGTM!

@baszalmstra baszalmstra merged commit a39a35d into conda:main Oct 21, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants