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

Proxito: always use subdomain for serving docs #10799

Merged
merged 5 commits into from
Oct 10, 2023
Merged

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Oct 5, 2023

Closes #10767


📚 Documentation previews 📚

@stsewd stsewd requested review from a team as code owners October 5, 2023 00:23
@stsewd stsewd requested a review from ericholscher October 5, 2023 00:23
Comment on lines -206 to -207
or "localhost" in request.get_host()
or "testserver" in request.get_host()
Copy link
Member Author

@stsewd stsewd Oct 5, 2023

Choose a reason for hiding this comment

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

These two checks are not really that useful, and they cause problems if your project has localhost in its name.

Copy link
Member

Choose a reason for hiding this comment

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

Hrm, I feel like it might cause some issues in tests, but if everythings looks OK, I agree this is a better approach.

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Glad to see this getting removed 🎉

docs/dev/settings.rst Outdated Show resolved Hide resolved
@@ -122,11 +113,6 @@ def resolve_path(
filename = self._fix_filename(filename)

parent_project, project_relationship = self._get_canonical_project_data(project)
cname = (
Copy link
Member

Choose a reason for hiding this comment

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

Do we not still need this logic to determine cname usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not when resolving the path, only when resolving the domain

Comment on lines -206 to -207
or "localhost" in request.get_host()
or "testserver" in request.get_host()
Copy link
Member

Choose a reason for hiding this comment

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

Hrm, I feel like it might cause some issues in tests, but if everythings looks OK, I agree this is a better approach.

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>
@stsewd stsewd merged commit 685a255 into main Oct 10, 2023
2 checks passed
@stsewd stsewd deleted the always-use-subdomain branch October 10, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxito: remove support for USE_SUBDOMAIN = False
2 participants