-
Notifications
You must be signed in to change notification settings - Fork 570
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
Add --save-as
param to record
#3576
Conversation
`--save-as` specifies the trace dir name (basename?), not the full path. Merged `output_trace_dir` in RecordFlags with `name` into new type `TraceOutputPath`. Changed all usage of `output_trace_dir` string to use this type instead. This PR is backwards compatible, in the sense that the old usage of the `-o` flag remains the same, if `--save-as` is not provided, i.e., will error out, if dir exists etc. If `--save-as` is provided together with `-o` the new behavior will happen instead, where the output dir will be the "root dir", thus, the user can save many traces there. If only `--save-as` is provided, record uses normal behavior, of setting the trace root dir to $RR_TRACE_DIR (or it's user provided env var). Naming of user provided dirs, follows old behavior, i.e. appending -0, -1, -2 etc to the trace dir. I think this is preferable - if some automated thing is recording something with a specific name provided, this makes it so the end user don't have to manage the file system (i.e. checking if that name is "taken" and having to do clean up before recording, etc.)
|
Why do you think it's not parsed? |
Never mind. I had entirely missed that it was specified in the OptionSpec. And I even grepped for it last night. goes to show, don't do things if you're tired. 😛 Either way, I had asked Roc about a feature like this and if it would be acceptable as a PR. I can think of at least one use case where this would be useful, where RR is used to record failing tests (automatically) and being able to specify a name, instead of just the application name (and instead of just a path, that has to be unique) and he OK'd it on Matrix. Let me know what you think of the PR. |
@theIDinside Would you mind rebasing this PR so it is more easily tested and integrated? |
Absolutely. I need to do the same with another PR so I will do that after work or this weekend. |
00efaa3
to
7bc19b3
Compare
@GitMensch |
7bc19b3
to
00efaa3
Compare
I screwed this one up. I will re-send PR. |
--save-as
specifies the trace dir name (basename?), not the full path.Merged
output_trace_dir
in RecordFlags withname
into new typeTraceOutputPath
.Changed all usage of
output_trace_dir
string to use this type instead.This PR is backwards compatible, in the sense that the old usage of the
-o
flag remains the same, if--save-as
is not provided, i.e., will error out, if dir exists etc. If--save-as
is provided together with-o
the new behavior will happen instead, where the output dir will be the "root dir", thus, the user can save many traces there.If only
--save-as
is provided, record uses normal behavior, of setting the trace root dir to $RR_TRACE_DIR (or it's user provided env var).Naming of user provided dirs, follows old behavior, i.e. appending -0, -1, -2 etc to the trace dir. I think this is preferable - if some automated thing is recording something with a specific name provided, this makes it so the end user don't have to manage the file system (i.e. checking if that name is "taken" and having to do clean up before recording, etc.) - especially since there's kind of functionality for that, already existing (just using the
-o
on it's own, though you have to specify a path, not a name)