Skip to content

Conversation

@s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Nov 29, 2024

DESCRIBE YOUR PR

Improves some sections of the docs based on feedback from @whitep4nth3r

IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs to go live.

  • Urgent deadline (GA date, etc.):
  • Other deadline:
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

@vercel
Copy link

vercel bot commented Nov 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 10:16am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Dec 4, 2024 10:16am
develop-docs ⬜️ Ignored (Inspect) Visit Preview Dec 4, 2024 10:16am

@s1gr1d s1gr1d self-assigned this Nov 29, 2024
@codecov
Copy link

codecov bot commented Nov 29, 2024

Bundle Report

Changes will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 10.2MB 9 bytes (-0.0%) ⬇️
sentry-docs-client-array-push 9.11MB 6 bytes (-0.0%) ⬇️

</Expandable>
<Expandable permalink title="pnpm: Resolving 'import-in-the-middle' external package errors">
<Expandable title="pnpm dev: Cannot find package 'import-in-the-middle'">
Copy link
Member

@andreiborza andreiborza Dec 2, 2024

Choose a reason for hiding this comment

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

q: Is this only an issue for pnpm dev?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, only in dev mode. Just tested it and building and running the build output works with pnpm

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, I wonder if we then should even bother with installing iitm in the wizard flow (getsentry/sentry-wizard#727)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's fine to install it, people will probably want to run Sentry in the dev mode as well. What about changing the prompt to something like this "To run Sentry in pnpm dev, 'import-in-the-middle' needs to be installed."

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to devs wanting to run Sentry in dev mode.

```

This step is necessary because Nuxt sets default values for source maps ([Nuxt docs](https://nuxt.com/docs/api/nuxt-config#sourcemap)), and the Sentry Nuxt Module keeps these settings when they are explicitly defined.
The option `'hidden'` works like `true`, but suppresses the source map reference comments that would be added to the end of each generated file in the build output.
Copy link
Member

Choose a reason for hiding this comment

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

m: I'd add a reason for why we want hidden over true, otherwise this info is just repeating someone else's docs.

@andreiborza
Copy link
Member

Screenshot 2024-12-02 at 13 09 10@2x

the formatting here seems off, could you also fix this please?

Copy link
Contributor

@lizokm lizokm left a comment

Choose a reason for hiding this comment

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

Thanks for adding! I left some language suggestions :)

@s1gr1d s1gr1d merged commit 8e588bd into master Dec 5, 2024
11 checks passed
@s1gr1d s1gr1d deleted the sig/improve-nuxt-docs branch December 5, 2024 10:20
Lms24 pushed a commit that referenced this pull request Dec 13, 2024
* feat(nuxt): Improve docs based on feedback

* add tilde

* add 'hidden' explanation

* delete org and project from wizard

* fix formatting

* liza review suggestions
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants