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

Avoid overwriting an existing robots.txt #246

Merged
merged 4 commits into from
Apr 30, 2019
Merged

Avoid overwriting an existing robots.txt #246

merged 4 commits into from
Apr 30, 2019

Conversation

jveillet
Copy link
Contributor

@jveillet jveillet commented Apr 3, 2019

Hi, and thank you for making this awesome Gem.

Here is a PR that attempts to do one thing: If I have already a robots.txt file with custom values at the root of the Jekyll site, then the Generator will take the content of this custom robots.txt and append it to the robots.txt template.

I am not sure it really fix the issue described in #189, so let me know if something doesn't feel right. Thanks for taking the time to look!

@ashmaroli
Copy link
Member

@jveillet Thank you for submitting this PR, but I think the proposed changes does more than necessary to achieve the desired result.
To me the suggestion in #189 (comment)
looks sufficient.
Have you tried implementing just that..?

@jveillet
Copy link
Contributor Author

@ashmaroli Thanks for reviewing. Maybe I've come too far with my implementation, I will modify the PR to just do what was suggested into the related issue.

For what I've tested so far the solution seems to work, I am trying to figure out how to modify the tests in accordance.

My only concern is that by doing this: OK we are not overriding anymore the user-defined robots.txt, but we are not either writing the sitemap URL into it, leaving the user to manually add it into her/his own robots.txt.

@ashmaroli
Copy link
Member

we are not either writing the sitemap URL into it, leaving the user to manually add it into her/his own robots.txt

Yes, that's indeed the primary intention here. If a user is maintaining a source-file for their robots.txt we (this plugin) shouldn't bother adding a proper robots file for them. The responsibility is entirely on the user.

The situation here is, say I have a site where all of my "standalone pages" like index.html, about-us.html, etc are sourced from files within a directory named _pages. If the same directory also contains a source-file that generates a file at path _site/robots.txt, then jekyll-sitemap shouldn't bother about its contents.
The maximum we should do is document that the following content in their robot's source file will help Jekyll include the proper URL to their sitemap:

---
---

Sitemap: {{ "sitemap.xml" | absolute_url }}

@jveillet
Copy link
Contributor Author

Sorry for the delay for fixing the test, it took me a while to realize I had to generate the tests in a different directory in order to make it work.

@jveillet
Copy link
Contributor Author

@ashmaroli Anything else I can do to help move this forward?

@ashmaroli ashmaroli changed the title Overwrite existing robots.txt Avoid overwriting an existing robots.txt Apr 25, 2019
@ashmaroli
Copy link
Member

@jveillet I've amended your commit messages to correctly reflect the intention of each commit. In addition to that, I've refactored the tests for this pull request to test valid scenarios.

You need not do anything more till further review.

@ashmaroli ashmaroli requested a review from a team April 25, 2019 04:14
@mattr-
Copy link
Member

mattr- commented Apr 30, 2019

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 7d73b35 into jekyll:master Apr 30, 2019
jekyllbot added a commit that referenced this pull request Apr 30, 2019
@jekyll jekyll locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants