feat(metrics): Add send-metric command#2063
Conversation
b40f9ce to
1a39b52
Compare
1a39b52 to
61ada35
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
I have only reviewed some of the files. Will continue reviewing the rest later
26ee405 to
d7b0ba8
Compare
|
Please also remember to rename this PR to "feat(metrics): ..." |
d7b0ba8 to
76c5cbc
Compare
b19bcb9 to
4815045
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Another partial review – I think I checked everything besides the integration tests. Mostly small changes/nitpicks, but I did find a bug related to string indexing which will need to be fixed
7bf8519 to
7cafaaf
Compare
iambriccardo
left a comment
There was a problem hiding this comment.
LGTM, I will leave the approval to Daniel since I don't have all the CLI context I would need to properly review this PR.
7cafaaf to
d205688
Compare
szokeasaurusrex
left a comment
There was a problem hiding this comment.
I added some comments, and requested some changes, primarily related to adding clearer user-facing documentation. This time, I looked through all the files (including integration tests) – some of the changes I requested are in the files changed for the integration tests.
Overall, this PR seems to be coming together really well, and the code looks overall very good!
006ead9 to
a716176
Compare
Add CLI command that can emit metrics. Fixes GH-2001
a716176 to
319733e
Compare
sl0thentr0py
left a comment
There was a problem hiding this comment.
just a couple of comments, rest looks good
a4b1867 to
74a1aa7
Compare
Add CLI command
send-metricfor emitting metrics to Sentry.Add new command-parser that uses clap's Derive API. Future commands should use this as the Derive API makes it:
-Easier to read, write, and modify arguments and commands.
-Easier to keep the argument declaration and reading of argument in sync.
-Easier to reuse shared arguments.
Fixes GH-2001