-
Notifications
You must be signed in to change notification settings - Fork 0
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 "sourcemaps upload" command using a mock call #54
Conversation
Error handling can be improved when integrating with the real API call
.requiredOption( | ||
'--realm <value>', | ||
'Realm for your organization (example: us0). Can also be set using the environment variable O11Y_REALM', | ||
process.env.O11Y_REALM |
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.
If we want to default as us0, we could replace with `process.env.O11Y_REALM || 'us0'
.summary(`Upload source maps to Splunk Observability Cloud`) | ||
.description(uploadDescription) | ||
.requiredOption( | ||
'--directory <path>', |
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.
Should there also be an option to upload a specific file ? Directory could contain multiple sourcemaps that may not be needed, or it may contain multiple versions of a sourcemap , if so it may not be effective to upload all of them ?
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.
This is a good suggestion for enhancement. We could have some wider conversation and add in a follow-up PR if needed.
Some other options could be to use additional option like --include
or --exclude
to let user refine the file selection
Formatting the file upload progress still needs improvement (currently prints using "bytes" as the unit)
Error handling can be improved when integrating with the real API call
Dry run can be implemented in future PR