-
Notifications
You must be signed in to change notification settings - Fork 0
add support for rewrite and ext options
#1
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdded two new configuration options to the action: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@aaronnorby adding you as reviewer because you wrote the original if there's someone more suited, let me know! |
|
@aaronnorby just a ping |
aaronnorby
left a comment
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.
sorry about the delayed review! lgtm
|
no apologies necessary, just following up on my open PRs! |
i was hoping to use this action in the chrome extension release automation, but it was missing some options we use in our hand written script for uploading source maps:
--no-rewriteand--ext.this PR adds new options, which already existed in the sentry CLI, they just weren't exposed in the action.
I think we could now replace the following script:
With the following workflow step:
Design decisions:
instead of adding a
noRewriteoption, i added arewritewhich can be set tofalse. this is more natural than a negative option.instead of
extaccepting a yml array (which on the surface is more idiomatic), it accepts a space separated string. the space separated string pattern is already used in the action (sourcemaps,projects), and apparently github stringifies yml arrays anyway when they get passed as options. so i opted for consistency