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

fix(docs): Update @astrojs/sitemap readme to clarify build output location #8844

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

evadecker
Copy link
Contributor

@evadecker evadecker commented Oct 15, 2023

Changes

  • This slightly modifies the language in the docs to clarify that /dist is the default output, but generated files may live elsewhere (a confusion that cost me a silly amount of time trying to debug).
  • I considered adding a note that the Vercel adapter will output to .vercel/output and that other adapters may output code to a place other than /dist, but this felt too in-the-weeds. If this is helpful to call out, happy to make edits!
  • Resolves @astrojs/sitemap does not always output sitemaps to dist/ docs#4919 (Not sure if Github will auto-close this PR since it's in a separate repo, but the sitemap docs get auto-generated from this README)
  • Also pairs with @silent1mezzo's improvements to the CLI text: Added OUTPUT dir to sitemap build command #8824

Testing

Docs-only change.

@evadecker evadecker requested a review from a team as a code owner October 15, 2023 23:44
@changeset-bot
Copy link

changeset-bot bot commented Oct 15, 2023

⚠️ No Changeset found

Latest commit: 4799801

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Oct 15, 2023
@evadecker evadecker changed the title minor: Update @astrojs/sitemap readme to clarify build output location docs: Update @astrojs/sitemap readme to clarify build output location Oct 15, 2023
@evadecker evadecker changed the title docs: Update @astrojs/sitemap readme to clarify build output location fix(docs): Update @astrojs/sitemap readme to clarify build output location Oct 15, 2023
@ematipico
Copy link
Member

ematipico commented Oct 16, 2023

  • (Not sure if Github will auto-close this PR since it's in a separate repo, but the sitemap docs get auto-generated from this README)

It seems it will :)
Screenshot 2023-10-16 at 11 57 08

@ematipico
Copy link
Member

@withastro/maintainers-docs this is your turf, please review :)

Copy link

@newtoallofthis123 newtoallofthis123 left a comment

Choose a reason for hiding this comment

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

lgtm 😄

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.

Thank you so much for this helpful restructuring, @evadecker! 🥳

It's a great tweak to the existing sentence to more properly identify the sitemap files. I'd like to work on the ending though, since we don't typically use em-dashes in docs. That probably means either sticking another clause on the end, or using parenthesis to include the extra information of the default output directory.

I left a comment with a suggestion below. See what you think, or if that gives you any better ideas. 🙌

packages/integrations/sitemap/README.md Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@evadecker
Copy link
Contributor Author

Thank you so much for this helpful restructuring, @evadecker! 🥳

It's a great tweak to the existing sentence to more properly identify the sitemap files. I'd like to work on the ending though, since we don't typically use em-dashes in docs. That probably means either sticking another clause on the end, or using parenthesis to include the extra information of the default output directory.

I left a comment with a suggestion below. See what you think, or if that gives you any better ideas. 🙌

I respect killing the em dash—I overuse it. :)

Great suggestion @sarah11918, applied!

@ematipico ematipico merged commit edc467d into withastro:main Oct 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@astrojs/sitemap does not always output sitemaps to dist/
5 participants