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

add theme's robots.txt to tera with name "robots.txt" #1722

Merged
merged 2 commits into from
Jan 11, 2022

Conversation

mwcz
Copy link
Contributor

@mwcz mwcz commented Jan 7, 2022

IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

This fixes #1721 by registering the theme's robots.txt template under the name "robots.txt" whereas it was previously being registered with the filepath, and later ignored during rendering.

Note: there are no tests for this PR at the moment. I'm looking into how and where to add some, but it might take me a bit.

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)? (docs are already correct)

@mwcz mwcz force-pushed the fix-robots.txt-in-theme branch from cee2067 to 6b3df91 Compare January 7, 2022 18:50
@Keats
Copy link
Collaborator

Keats commented Jan 7, 2022

Thanks!

Note: there are no tests for this PR at the moment. I'm looking into how and where to add some, but it might take me a bit.

Probably an integration test: add a robots.txt file in test_site/themes/sample/ and then add an assert in components/site/tests/site.rs

@mwcz
Copy link
Contributor Author

mwcz commented Jan 10, 2022

I tried this out but even if I add a test_site/themes/sample/robots.txt, the test site already has a templates/robots.txt which takes precedence. I've been trying to figure out a way to test this scenario without affecting the existing tests for templates/robots.txt and so far nothing has worked very well. Here's what I've toyed with:

  1. make a slimmed-down copy of test_site to test this scenario → no, too much duplication
  2. in the test, use build_site_with_setup and remove templates/robots.txt somehow before the site is rendered, allowing test_site/themes/sample/robots.txt to shine through. It seems like this could work but I haven't found an API yet that would allow that, after poking around Site, Library, and Tera references available from build_site_with_setup.

Do either of these sound good, or is there some other cleaner approach I'm missing?

@Keats
Copy link
Collaborator

Keats commented Jan 10, 2022

Hmm, easier test setup is definitely something that needs work. Can you just add a TODO above the code saying it needs a test and a link to that PR? We'll add a test after some refactoring

@mwcz
Copy link
Contributor Author

mwcz commented Jan 10, 2022

TODO added.

@Keats
Copy link
Collaborator

Keats commented Jan 10, 2022

Just to confirm before merging, you did test manually that it worked correct?

@mwcz
Copy link
Contributor Author

mwcz commented Jan 10, 2022

Yeah, I tested it on the blog @t3link posted in #1721.

https://asciinema.org/a/aSi4IQxolor2IYUkMxeLiflbi

@Keats Keats merged commit 3caf441 into getzola:next Jan 11, 2022
@Keats
Copy link
Collaborator

Keats commented Jan 11, 2022

Thanks!

@mwcz mwcz deleted the fix-robots.txt-in-theme branch January 11, 2022 14:09
Keats pushed a commit that referenced this pull request Jan 22, 2022
* add theme's robots.txt to tera with correct name

* add TODO reminder to add tests
Keats pushed a commit that referenced this pull request Jan 23, 2022
* add theme's robots.txt to tera with correct name

* add TODO reminder to add tests
thomasantony pushed a commit to thomasantony/zola that referenced this pull request Sep 17, 2022
* add theme's robots.txt to tera with correct name

* add TODO reminder to add tests
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.

2 participants