Conversation
|
Wasn't sure if |
krasznaa
left a comment
krasznaa
left a comment
There was a problem hiding this comment.
The C++ code is definitely good this way. @stephenswat will have to have a look at the Python code. Since as I understand it, the code in this repository is just a copy of what he actually uses to run our benchmarks. (I.e. he'll need to pull this update to his copy.)
| "Commit is not a child of %s; event order is random", | ||
| DETERMINISTIC_ORDER_COMMIT[:8], | ||
| ) | ||
| profile_args.append("--deterministic=0") |
There was a problem hiding this comment.
Please don't add this here; this will cause the code to issue an unknown flag to older commits which do not know how to handle that flag. Simply remove the flag from the positive half of the if clause.
There was a problem hiding this comment.
As a matter of fact, please create two commits: put all the C++ changes in one, then record the commit hash of that commit into this if-else clause. That will be the most robust solution.
Actually all the CI jobs use the code from the repository directly, so this should be fine. 🙂 |
|
3 participants
Probably a less surprising default