Skip to content

feat(nodejs): Deprecate frameContextLines option and move to integration #4746

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

Merged
merged 6 commits into from
Feb 24, 2022

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Feb 18, 2022

These are docs updates to cover the changes in this PR:
getsentry/sentry-javascript#3729

  • Adds a new ContextLines integration which has a frameContextLines option
  • Marks frameContextLines in node options as deprecated and links to the integration

@vercel
Copy link

vercel bot commented Feb 18, 2022

@timfish is attempting to deploy a commit to the Sentry Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Feb 22, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sentry/sentry-docs/E3Mauqm6b4zEkxMTGuqb4oLRtc3g
✅ Preview: https://sentry-docs-git-fork-timfish-node-framecontextlines.sentry.dev

@timfish
Copy link
Collaborator Author

timfish commented Feb 24, 2022

I added the version number but now it appears to be failing on broken links for what I have added!

@AbhiPrasad
Copy link
Member

:ohno: there's some anchor issues. Maybe configuration/integrations/default-integrations/#contextlines doesn't exist for all node platforms?

@timfish
Copy link
Collaborator Author

timfish commented Feb 24, 2022

Maybe configuration/integrations/default-integrations/#contextlines doesn't exist for all node platforms?

I thought this would be enough:

image

Perhaps the link checker doesn't respect conditional sections?

@AbhiPrasad
Copy link
Member

Perhaps the link checker doesn't respect conditional sections

It should since we run it on the built assets. @imatwawana, have you come across this before?

@timfish
Copy link
Collaborator Author

timfish commented Feb 24, 2022

Maybe I can link directly to the node docs so the link is always valid?

@imatwawana
Copy link
Contributor

Perhaps the link checker doesn't respect conditional sections

It should since we run it on the built assets. @imatwawana, have you come across this before?

I'm going to try a couple things to see if it fixes the issue.

@imatwawana
Copy link
Contributor

Maybe I can link directly to the node docs so the link is always valid?

Generally, we want to avoid hard coded links where relative linking should work. It looks like my fix worked; there was just an extra forward slash at the end of the link that shouldn't have been there.

@timfish
Copy link
Collaborator Author

timfish commented Feb 24, 2022

Oh nice. Thank you!

@AbhiPrasad AbhiPrasad merged commit e62e7d5 into getsentry:master Feb 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2022
@timfish timfish deleted the node-frameContextLines branch May 24, 2022 13:08
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.

3 participants