-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Show toolchain column when multiple toolchains are used #2121
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
base: master
Are you sure you want to change the base?
Conversation
5375828 to
6b698f7
Compare
adamsitnik
left a comment
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 am afraid this is trickier that it looks like.
Currently when I run
cd samples\BenchmarkDotNet.Samples
dotnet run -c Release -f net6.0 --filter *IntroBasic.Sleep --join --runtimes net462 net6.0 --job dryI get:
With your changes the toolchain column becomes visible again:
We want the toolchain column to be visible only when there are at least two benchmarks that use the same Runtime, but different Toolchain.
ad42f4c to
583bb66
Compare
|
It looks that we should never display the
I think we should disallow setting |
1380f38 to
27aec58
Compare
|
|
||
| [Theory] | ||
| [InlineData("nativeaot6.0", "NativeAOT 6.0", "ILCompiler 6.0.0-*")] | ||
| [InlineData("nativeaot7.0", "NativeAOT 7.0", "Latest ILCompiler")] |
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.
Is Latest ILCompiler the desired toolchain name? Should it be ILCompiler 7.0.0-*?
|
Ready for review. The only problem I found is: BenchmarkDotNet/tests/BenchmarkDotNet.Tests/Columns/JobColumnsApprovalTests.cs Lines 92 to 97 in aa8ef43
I didn't dare to change a default toolchain. |
aa8ef43 to
371ee5d
Compare
Done
|
371ee5d to
2c99d26
Compare
Some possible improvements:1) The
|


The 3rd commit shows what's changed.
I removed
Summary.IsMultipleRuntimesbecause it is actually redundant.Now the
Toolchaincolumn will be showed only when using.WithToolchain()method.I tried shortening the approval filenames, but they are still long