-
Notifications
You must be signed in to change notification settings - Fork 18
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
Basic setup for benchmarks #74
Conversation
9978662
to
9d45857
Compare
@duesee can you review this so we can get it in? |
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.
Generally this looks good to me!
What I am unsure about: Similar to the case that we have to think about running build --instrument
before test --instrument
we now introduce colliding workflows. From my understanding, it can happen that a build --benchmarks
run overrides the output of a build --tests
run, which may lead to confusing errors. Thus, we need to be a bit careful what to run when.
Would it be difficult to output the benchmarks into a separate folder build/Debug/benchmark
? If so, I still think it is okay for now as it is more important to get some numbers.
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.
Yeah this is certainly not done yet. It's just adding the basic framework.
Let's get this in and address all the issues once the testing is done.
3d4feac
to
8452615
Compare
This needs cleanup and better integration. It's only a basic setup to show how it is supposed to work.
1de7d3f
to
94b470a
Compare
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've rebased on main. Let's get this in 👍
thanks, but please stop force pushing on other people's branches |
This needs cleanup and better integration. It's only a basic setup to show how it is supposed to work.
#15 #36