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

Use the first token of --chruby as the default name #244

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Aug 1, 2023

What's this?

I use ./run_benchmarks.rb --chruby 'before::before --yjit;after::after --yjit' to compare two revisions. I use this so often that I want to make it a bit shorter.

This PR allows you to use --chruby 'before --yjit;after --yjit' while keeping the names before and after, not before --yjit and after --yjit as the current implementation would do.

Why this change is safe

When you compare the interpreter and YJIT, you simply don't need to use --chruby in the first place. So, when you use --chruby, you most likely specify different chruby binaries. Therefore it makes sense to use the first token of a --chruby option as the name of the binary in most cases.

Even if you want to specify the same chruby binary with different options in --chruby for whatever reason, you can still explicitly write name:: before specifying the chruby name and opptions.

Also, I didn't update README.md because use cases written there work as is, and the use of --chruby is already explained well. This change is fairly backward-compatible in that sense too.

@maximecb maximecb merged commit ae160f0 into Shopify:main Aug 2, 2023
2 checks passed
@k0kubun k0kubun deleted the chruby-name branch August 2, 2023 16:18
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