Skip to content

Conversation

@WickyNilliams
Copy link

@WickyNilliams WickyNilliams commented Nov 7, 2025

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-rewrite and --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:

SENTRY_PROJECT=$project yarn sentry-cli releases new --finalize $release

echo - "${BLUE}Uploading source maps${END_COLOR}"

SENTRY_PROJECT=$project yarn sentry-cli releases files \
$release upload-sourcemaps --no-rewrite --url-prefix "~/" --ext js --ext map ./build

With the following workflow step:

- uses: ashbyhq/sentry-action-release@master
  with:
    version: ${{ github.sha }}
    sourcemaps: './build'
    url_prefix: '~/'
    rewrite: false
    ext: 'js map'
  env:
    SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
    SENTRY_ORG: ashby
    SENTRY_PROJECT: 'chrome_extension'

Design decisions:

instead of adding a noRewrite option, i added a rewrite which can be set to false. this is more natural than a negative option.

instead of ext accepting 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

@coderabbitai
Copy link

coderabbitai bot commented Nov 7, 2025

📝 Walkthrough

Walkthrough

Added two new configuration options to the action: rewrite (boolean, default true) to control sourcemap rewriting, and ext (space-separated extensions list) to specify allowed source map file extensions. Updates include action definition, implementation across TypeScript and JavaScript modules, tests, and documentation.

Changes

Cohort / File(s) Summary
Configuration & Documentation
README.md, action.yml
Added rewrite and ext input parameters with descriptions and defaults to the action manifest and usage documentation. Included workflow example demonstrating both options.
Source Implementation
src/options.ts, src/main.ts
Introduced getExtOption() function to parse space-separated extensions into array. Updated main flow to retrieve both rewrite and ext options and pass them into sourceMapOptions.
Compiled Output
lib/options.js, lib/main.js
Compiled versions of TypeScript source files with corresponding new functions and option threading.
Test Coverage
__tests__/main.test.ts
Added test suite for getExtOption() covering undefined input, array return, and whitespace trimming/empty token filtering.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review consistency between TypeScript sources and compiled JavaScript outputs
  • Verify option threading from retrieval through to CLI invocation in both src/main.ts and lib/main.js
  • Confirm test coverage for getExtOption() edge cases (whitespace handling, empty strings)

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding two new configuration options (rewrite and ext) to the action.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description clearly explains the motivation, what options are being added, and design decisions for implementing rewrite and ext options.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nick/add-rewrite-and-ext-options

Comment @coderabbitai help to get the list of available commands and usage tips.

@WickyNilliams
Copy link
Author

@aaronnorby adding you as reviewer because you wrote the original upload_sourcemaps script i'm hoping to replace here

if there's someone more suited, let me know!

@WickyNilliams
Copy link
Author

@aaronnorby just a ping

Copy link

@aaronnorby aaronnorby left a 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

@WickyNilliams
Copy link
Author

no apologies necessary, just following up on my open PRs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants