Allow sharing traces via MAGIC_TRACE_SHARE_COMMAND#132
Open
Xyene wants to merge 1 commit intojanestreet:masterfrom
Open
Allow sharing traces via MAGIC_TRACE_SHARE_COMMAND#132Xyene wants to merge 1 commit intojanestreet:masterfrom
MAGIC_TRACE_SHARE_COMMAND#132Xyene wants to merge 1 commit intojanestreet:masterfrom
Conversation
Contributor
|
Port configurability is a big deal! We all use the same machine at work to do magic-trace stuff. That said, I'm fine with adding it back in a separate feature. |
Tested with `MAGIC_TRACE_SHARE_COMMAND=/usr/bin/echo`. Also fixed a bug where we were unconditionally serving the UI if the Perfetto base dir was specified. We lose port configurability, but it's not a big deal and we can add it back as a separate flag if we want to.
Member
Author
|
I'm not sure which I like more:
|
Contributor
|
My heart says (2, but start at 1024), but my brain says (1). S'up to you. |
cgaebel
requested changes
Apr 8, 2022
| flag | ||
| "serve" | ||
| no_arg | ||
| ~doc:[%string " Serves the magic-trace UI on port %{port#Int})"] |
| ~args:[ Filename_unix.realpath store_path ] | ||
| () | ||
| in | ||
| Core.printf "%s%!" output |
Contributor
There was a problem hiding this comment.
This won't show output until the command ends, and it might also swallow stderr? I'd use the cruddy process APIs to transfer stdout/stderr of the subprocess to magic-trace's.
|
|
||
| let maybe_param = | ||
| Option.map Env_vars.share_command_filename ~f:(fun share_command_filename -> | ||
| let%map_open.Command share = flag "share" no_arg ~doc:"Share trace." in |
Contributor
There was a problem hiding this comment.
That help text could say more. If you don't want it to be too long, I'm happy with the docs just being magic-trace.org/w/share and then we explain how sharing works on the wiki somewhere.
Serve could arguably work the same way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tested with
MAGIC_TRACE_SHARE_COMMAND=/usr/bin/echo.Also fixed a bug where we were unconditionally serving the UI if the
Perfetto base dir was specified. We lose port configurability, but it's
not a big deal and we can add it back as a separate flag if we want to.