Skip to content

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Aug 8, 2025

Adds support for uploading dart symbol maps that are associated via debug id extracted from the provided debug file

example usage:

sentry-cli dart-symbol-map upload --org my-org --project my-proj /path/to/mapping /path/to/debug-file

Tests:

  • integration tests
  • manual tests of uploading to the test sdk repo on sentry

Part of getsentry/sentry-dart#2805

@buenaflor buenaflor requested review from szokeasaurusrex and a team as code owners August 8, 2025 14:02
@szokeasaurusrex
Copy link
Member

I'll try to do a fuller review before I head into vacation from Tuesday afternoon – but in case I don't get to, and you request review from someone else, just want to leave an initial thought here:

I would suggest that instead of naming this command sentry-cli upload-dart-symbol-map, we name it sentry-cli dart-symbol-map upload.

The reason is that this aligns with the APIs for debug-files upload and sourcemaps upload. Also, this would then mean we have a dart-symbol-map subcommand, with an upload command nested below. That way, if we ever want to add more functionality related to Dart symbol maps, we can also add this functionality under the dart-symbol-map subcommand.

@buenaflor
Copy link
Contributor Author

Thanks, will update the command

@buenaflor buenaflor changed the title feat(dart): add upload-dart-symbol-map command feat(dart): add dart-symbol-map upload command Aug 11, 2025
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As currently written, this command will use way more memory than needed. Please address these, and also restructure the code as requested.

Feel free to dismiss my review once fixing these, if I am still on vacation

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@buenaflor buenaflor requested a review from loewenheim August 13, 2025 09:06
Copy link
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Have you verified that it works with Sentry?

Co-authored-by: Sebastian Zivota <loewenheim@users.noreply.github.com>
@buenaflor
Copy link
Contributor Author

thx, yes works as expected

@buenaflor buenaflor enabled auto-merge (squash) August 13, 2025 09:50
@buenaflor buenaflor disabled auto-merge August 13, 2025 11:12
@buenaflor buenaflor dismissed szokeasaurusrex’s stale review August 13, 2025 11:13

approved review by another reviewer

@buenaflor buenaflor merged commit 4e7d0e4 into master Aug 13, 2025
31 checks passed
@buenaflor buenaflor deleted the feat/dart-symbol-map branch August 13, 2025 11:14
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