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: document locals in adapter API #3685

Merged
merged 11 commits into from
Jul 18, 2023
Merged

Conversation

ematipico
Copy link
Member

What kind of changes does this PR include?

  • New or updated content

Description

Relevant PR: withastro/astro#7385

This PR adds a new paragraph to the app.render heading. Unfortunately, I don't know enough to explain the second argument (routeData), so I left a TODO there.

It would be great if we could document it in this PR, but I would need some help.

@netlify
Copy link

netlify bot commented Jul 10, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 6654313
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64b66492c8cd050007f9457a
😎 Deploy Preview https://deploy-preview-3685--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@sarah11918 sarah11918 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 this, @ematipico! I took a shot at using links to content we already have to see if this could work for the section. What do you think? Do you think this is complete enough?

src/content/docs/en/reference/adapter-reference.mdx Outdated Show resolved Hide resolved
src/content/docs/en/reference/adapter-reference.mdx Outdated Show resolved Hide resolved
src/content/docs/en/reference/adapter-reference.mdx Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <[email protected]>
@ematipico
Copy link
Member Author

@sarah11918 thank you for the review, everything looks good to me!

Copy link
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

Small nit, overall LGTM!

src/content/docs/en/reference/adapter-reference.mdx Outdated Show resolved Hide resolved
@sarah11918 sarah11918 added the improve documentation Enhance existing documentation (e.g. add an example, improve description) label Jul 10, 2023
@ematipico
Copy link
Member Author

@matthewp could you please advice on how we can explain routeData (second argument) of the render function?

@matthewp
Copy link
Contributor

That's a lot of stuff to document. Do we have any way to point that arg to a type that's defined?

@ematipico
Copy link
Member Author

That's a lot of stuff to document. Do we have any way to point that arg to a type that's defined?

I was thinking to document only when/why an adapter can pass a RouteData, and a brief example.

@ematipico
Copy link
Member Author

@sarah11918 @Yan-Thomas added a small paragraph about the second optional parameter.

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thanks, @ematipico ! Nice to have an example here!

@ematipico ematipico merged commit b6e6202 into main Jul 18, 2023
@ematipico ematipico deleted the feat/adapters-locals-plt-497 branch July 18, 2023 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants