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

[docs] Update Current Example for site: in the Configuration Reference #9675

Closed
wants to merge 1 commit into from

Conversation

VoxelMC
Copy link
Contributor

@VoxelMC VoxelMC commented Jan 12, 2024

Changes

Docs Update
The currently displayed setting for the site: configuration property does not contain a trailing slash, despite the change to this behaviour in v2 (#5604, #5608).

This closes withastro/docs#6255.

Testing

This is only a docs change

Docs

/cc @withastro/maintainers-docs for feedback!

Copy link

changeset-bot bot commented Jan 12, 2024

⚠️ No Changeset found

Latest commit: 03a8ad3

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 pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Jan 12, 2024
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

I don't think we need to change this. The suggestion to add a trailing slash was because if you want to keep compat like before, you can add a trailing slash to do so. For new projects, you can choose to have a trailing slash or not.

@ronan-smith
Copy link

ronan-smith commented Jan 12, 2024

@bluwy I don't believe the issue can be solved by adding or removing the trailing slash, however, the documentation shows an example without one but the language in the migration guide suggests that one should be added.

Surely there should be some additional guidance in the documentation regarding the implications of including/excluding the trailing slash?

Even now I am unsure if new projects should have one or not.

@bluwy
Copy link
Member

bluwy commented Jan 12, 2024

The only implication if you have or not have the trailing slash is that the value of import.meta.env.SITE is with or without the trailing slash that you specified. There aren't any downsides of using either that's worth mentioning IMO.

For new projects, you wouldn't be reading the v2 migration guide either, so I don't think it is confusing. The suggestions in the v2 migration guide are only for projects migrating to v2, not a general suggestion for all projects.

@ronan-smith
Copy link

@bluwy Thank you for taking the time to explain.

@ematipico
Copy link
Member

@bluwy should we close this PR? I don't understand what are the changes you requested from OP

@bluwy
Copy link
Member

bluwy commented Jan 19, 2024

Yeah we can close this 👍

@bluwy bluwy closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflicting/confusing "site" description in the Configuration Reference
5 participants