-
Notifications
You must be signed in to change notification settings - Fork 108
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
Beacon generates robots.txt
per site
#700
Conversation
make it backwards compatible, ie: works even with a single Endpoint (the default setup)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Just a suggestion to disallow the admin prefix.
@@ -0,0 +1,3 @@ | |||
# http://www.robotstxt.org |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could disallow the admin prefix here as well. See https://github.com/BeaconCMS/beacon_live_admin/blob/f8ff1621253c30c5f143655c11acf1116fa1c1e4/lib/beacon/live_admin/router.ex#L88
We need to use scoped_path
to build the full path including the scope
path. So if that attribute is present in the router we could include it as Disallow: <%= @admin_prefix %>
otherwise just ignore the line since the admin might not even be included in the router.
And we'll need at least a simple test because that function might change and we need a failing test if that happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could disallow the admin prefix here as well
I don't think that will be included. This PR only uses the beacon_site
macro for generation
Resolves #219
Depends on #694, #687