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

Verify r version matches during activate #136

Merged
merged 4 commits into from
Mar 5, 2025
Merged

Verify r version matches during activate #136

merged 4 commits into from
Mar 5, 2025

Conversation

wes-a2ai
Copy link
Contributor

@wes-a2ai wes-a2ai commented Mar 3, 2025

Minimal rv info with --r-version flag to allow for easy R version comparison

@wes-a2ai wes-a2ai requested review from dpastoor and Keats March 3, 2025 18:02
@wes-a2ai
Copy link
Contributor Author

wes-a2ai commented Mar 3, 2025

@dpastoor, If you can review the edited content for activate.R in consts to make sure the R code makes sense (i.e. ordering of the messages makes sense)

src/main.rs Outdated
let context = CliContext::new(&cli.config_file)?;

if r_version {
println!("{}", context.config.r_version());
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want to show context.config.r_version().original I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what the display trait of Version does

impl fmt::Display for Version {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.original)
}
}

Copy link
Member

@dpastoor dpastoor left a comment

Choose a reason for hiding this comment

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

minor tweaks but like the idea!

@wes-a2ai
Copy link
Contributor Author

wes-a2ai commented Mar 4, 2025

r.version.check.mp4

@Keats Keats force-pushed the activate_basic_info branch from 124be6c to c1f87ef Compare March 5, 2025 13:53
@Keats Keats merged commit 9461daa into main Mar 5, 2025
6 checks passed
@Keats Keats deleted the activate_basic_info branch March 5, 2025 14:01
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.

3 participants