Skip to content

Conversation

@jeckersb
Copy link
Collaborator

The rand crate was bumped from 0.8 -> 0.9 in 7804be9

Signed-off-by: John Eckersberg jeckersb@redhat.com

The rand crate was bumped from 0.8 -> 0.9 in 7804be9

Signed-off-by: John Eckersberg <jeckersb@redhat.com>
@bootc-bot bootc-bot bot requested a review from gursewak1997 November 14, 2025 14:22
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the rand crate from version 0.8 to 0.9 and fixes the resulting deprecation warnings in xtask. The changes are correct and achieve the goal of the PR. I've added one suggestion to make the random string generation more idiomatic using features from the rand crate.

Comment on lines +494 to 501
let mut rng = rand::rng();
const CHARSET: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789";
(0..8)
.map(|_| {
let idx = rng.gen_range(0..CHARSET.len());
let idx = rng.random_range(0..CHARSET.len());
CHARSET[idx] as char
})
.collect()
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While this is a correct fix for the deprecation warning, this implementation can be made more idiomatic and concise by using SliceRandom::choose to pick a random character directly from the CHARSET.

Here's how you could refactor the function body:

const CHARSET: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789";
(0..8)
    .map(|_| *CHARSET.choose(&mut rand::rng()).unwrap() as char)
    .collect()

This would require adding use rand::seq::SliceRandom; at the top of the file. Since this change is outside the diff, I'm providing this as a suggestion for you to consider.

@jeckersb jeckersb enabled auto-merge (rebase) November 14, 2025 14:32
@jeckersb jeckersb merged commit 71dc8e5 into bootc-dev:main Nov 14, 2025
34 of 38 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