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.

Sign up for GitHub

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

Jump to bottom

[Content collections] Add .astro directory setup #2313

Merged
merged 8 commits into from
Jan 12, 2023
Merged

[Content collections] Add .astro directory setup #2313

merged 8 commits into from
Jan 12, 2023

Conversation

bholmesdev
Copy link
Contributor

What kind of changes does this PR include?

  • New or updated content

Description

  • Add .astro directory explanation and setup
    • tip to add to gitignore
    • note on reference type setup

@netlify
Copy link

netlify bot commented Jan 6, 2023 •
edited
Loading

✅ Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 6890d2e
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/63c04702c181cd00080b428f
😎 Deploy Preview https://deploy-preview-2313--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 settings.

@bholmesdev bholmesdev mentioned this pull request Jan 6, 2023
Merged
@delucis
Copy link
Member

delucis commented Jan 9, 2023

This might change IIRC? Remembering https://discord.com/channels/830184174198718474/1060980519988367492. Won’t review until the dust has settled 🙌

@sarah11918 sarah11918 marked this pull request as draft January 9, 2023 17:42
@sarah11918
Copy link
Member

Just converted to draft for safety, since there's still some discussion in the upstream PR!

@bholmesdev
Copy link
Contributor Author

bholmesdev commented Jan 10, 2023 •
edited
Loading

@delucis @sarah11918 Discussed with core and decided this is the final API ™️

I want to clarify this documented env.d.ts update is performed automatically by all our CLI commands. I only included the docs for completeness if your project does not use the src/env.d.ts file our templates provide. Let me know if these docs make that clear, or if they could be restructured.

@bholmesdev bholmesdev marked this pull request as ready for review January 10, 2023 13:48
@bholmesdev bholmesdev force-pushed the edit/add-dot-astro-to-content-collections branch from 1692653 to a8d3446 Compare January 10, 2023 18:11
sarah11918
sarah11918 reviewed Jan 10, 2023
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

dammit stupid pending comments

src/pages/en/guides/content-collections.mdx Outdated Show resolved Hide resolved
sarah11918
sarah11918 approved these changes Jan 10, 2023
View reviewed changes
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.

I cannot argue with this! (Now, to figure out why the deploy preview failed? 😅 )

@sarah11918
Copy link
Member

Re-ran the Netlify deploy preview, just to be sure, but I don't see any reason why this should fail.

@bholmesdev
Copy link
Contributor Author

Hm, looks like an issue fetching remote fonts. Not seeing this locally either... any way to push up a cached font instead?

@sarah11918 sarah11918 added the add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. label Jan 10, 2023
@sarah11918
Copy link
Member

The latest deploy preview worked, and our local checks pass. So I think it's just safe to merge? If you wanted to be EXTRA safe, you could make one more trivial commit to rerun everything, and I bet it would be fine.

@sarah11918
Copy link
Member

I know! Merge z.object first, then come back here and update branch, then I bet everything Just Works ™️

delucis
delucis approved these changes Jan 11, 2023
View reviewed changes
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

LGTM! And agree with Sarah’s suggestion of merging #2335 first and updating branch just for the peace of mind of green checks.

@bholmesdev bholmesdev mentioned this pull request Jan 11, 2023
Closed
@sarah11918
Copy link
Member

Looking 🔥, Ben!

@delucis
Copy link
Member

delucis commented Jan 12, 2023

Ready to merge!

@bholmesdev bholmesdev merged commit 5c52a20 into main Jan 12, 2023
@bholmesdev bholmesdev deleted the edit/add-dot-astro-to-content-collections branch January 12, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarah11918 sarah11918 sarah11918 approved these changes

@delucis delucis delucis approved these changes

Assignees
No one assigned
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bholmesdev @delucis @sarah11918