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

ref(repo): Add @sentry-internal namespace to rrweb package names #17

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jan 19, 2023

This PR renames the rrweb packages by prepending our @sentry-internal namespace. I opted to take the internal one over the usual @sentry namespace as I find it more fitting for the purpose we want to release these packages for.

Note: I'm opening this PR against a publish-fork branch so we can continue adding fixes and patching rrweb manually until these packages are released.

ref #15

@@ -55,6 +55,7 @@ for (let config of baseConfigs) {
name: config.name,
format: 'iife',
file: pkg.unpkg.replace(pkg.name, config.path),
extend: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only deliberate change I made: After changing the namespace, rollup threw an error that the package name is illegal unless I add this option. Tbh I'm not entirely sure what the problem here is but I don't think we need this particular build config anyway.

@Lms24
Copy link
Member Author

Lms24 commented Jan 19, 2023

Hmm looks like prettier wasn't run on a lot of files... should we discard the autoformat changes?

@Lms24 Lms24 requested review from mydea and billyvg January 19, 2023 15:16
@Lms24 Lms24 changed the title ref(repo): Add '@sentry-internal' namespace to rrweb package names ref(repo): Add @sentry-internal namespace to rrweb package names Jan 19, 2023
@mydea
Copy link
Member

mydea commented Jan 19, 2023

Looks good to me! IMHO we can merge the prettier stuff as well, otherwise it'll come at a later point. Or, maybe we make one PR first with just the prettier changes, then a second one with the package changes?

@Lms24
Copy link
Member Author

Lms24 commented Jan 19, 2023

Or, maybe we make one PR first with just the prettier changes, then a second one with the package changes?

Sounds good to me 👍

@Lms24 Lms24 force-pushed the lms-publish-rename branch from 3ef18f5 to 12ba25b Compare January 19, 2023 17:52
@Lms24 Lms24 requested a review from mydea January 19, 2023 17:54
Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

using the internal namespace makes sense to me, we are not intending for the general public to be using this package.

@Lms24 Lms24 merged commit 7826783 into publish-fork Jan 20, 2023
@Lms24 Lms24 deleted the lms-publish-rename branch January 20, 2023 14:49
Lms24 added a commit that referenced this pull request Jan 23, 2023
rename the rrweb packages by prepending our `@sentry-internal`
namespace. I opted to take the internal one over the usual @sentry
namespace as I find it more fitting for the purpose we want to release
these packages for.
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