Skip to content
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

feat(sourcemaps): Add --use-debug-ids flag to force Artifact Bundles usage #1557

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

kamilogorek
Copy link
Contributor

Name is up for discussion.

  • remove SENTRY_FORCE_ARTIFACT_BUNDLES
  • remove forceArtifactBundles from JS API in favor of useDebugIds
  • restore chunk options endpoint method to the old, non-overriding behavior
  • add --use-debug-ids flag that overrides the server capability

@kamilogorek kamilogorek changed the title feat(inject): Add --use-debug-ids flag to force Artifact Bundles usage feat(sourcemaps): Add --use-debug-ids flag to force Artifact Bundles usage Mar 31, 2023
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

JS part is approved

@Jesse-Box
Copy link

Jesse-Box commented Mar 31, 2023

@kamilogorek @mitsuhiko @iambriccardo

Since we print the Bundle Type: Artifact Bundle after the upload command has been triggered. Wouldn't it make more sense to name the flag something like: -type-artifact?

Another question; is there any other flag a user can use that isn't interoperable with this new flag?

@iambriccardo
Copy link
Member

The problem with --type-artifact is that you imply that there is a default value when the user wants to use the old one or we will require it to be always specified.

@kamilogorek
Copy link
Contributor Author

Another question; is there any other flag a user can use that isn't interoperable with this new flag?

Good questions, because if yes, then we can actually model this inside flags structure (the lib we use allows to make one flag prevent use of other flag)

Copy link
Member

@iambriccardo iambriccardo left a comment

Choose a reason for hiding this comment

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

LGTM

src/commands/sourcemaps/upload.rs Outdated Show resolved Hide resolved
Co-authored-by: Riccardo Busetti <riccardob36@gmail.com>
@kamilogorek kamilogorek enabled auto-merge (squash) April 3, 2023 07:49
@kamilogorek kamilogorek merged commit 412d0b3 into master Apr 3, 2023
@kamilogorek kamilogorek deleted the debug-id-flag branch April 3, 2023 07:53
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.

5 participants