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

Disallow robots in non-production #175

Merged
merged 2 commits into from
Dec 23, 2018
Merged

Disallow robots in non-production #175

merged 2 commits into from
Dec 23, 2018

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Nov 12, 2018

Fixes #172. Got the example from https://github.com/twbs/bootstrap/blob/110c7f4cc0c5693583e0e69bdde3465c934b6e58/site/robots.txt

The question is: does netlify actually set environment: production when building for production/master?

@xPaw xPaw requested a review from astorije November 12, 2018 11:30
@astorije
Copy link
Member

Preview of your commit: https://5be9646fe47085303512ed78--thelounge.netlify.com/robots.txt
Preview of my dummy commit after I added the JEKYLL_ENV env variable (with production value) to our Netlify config: https://5bea6f3a40e2a44868ff04aa--thelounge.netlify.com/robots.txt

It seems to have worked to me.
We should probably move it to a netlify.toml file we'd put in the repo instead of a setting in their UI, but for now it will do. I wasn't sure which way you'd prefer so I went with what doesn't alter the repo for now.

@astorije
Copy link
Member

Wondering, by "non-production", did you intend this to differentiate builds for main website vs. deploy previews / next website? If so then the environment variable way set to production wouldn't work, but they do have a $CONTEXT env var that would work. Not sure how to leverage it though.

@xPaw
Copy link
Member Author

xPaw commented Nov 13, 2018

Wondering, by "non-production", did you intend this to differentiate builds for main website vs. deploy previews / next website?

Yes.

If so then the environment variable way set to production wouldn't work

Why did you approve the PR then, is there something else required to be set?

I see you set Build environment variables, but wouldn't that apply to all deploys/previews?

@xPaw
Copy link
Member Author

xPaw commented Nov 13, 2018

There's this plugin: https://github.com/jayvdb/jekyll-netlify

Which seems to set jekyll env based on netlify context.

@astorije
Copy link
Member

astorije commented Dec 5, 2018

Why did you approve the PR then, is there something else required to be set?

Because I initially thought it was to distinguish between dev and non-dev. I think. That was a while ago.

I see you set Build environment variables, but wouldn't that apply to all deploys/previews?

Correct.

There's this plugin: https://github.com/jayvdb/jekyll-netlify

I did see this, do you want to play with it?

@xPaw
Copy link
Member Author

xPaw commented Dec 5, 2018

I did see this, do you want to play with it?

I don't have ruby installed (nor can I be bothered to install it, haha) to update the gemfile correctly.

@astorije
Copy link
Member

astorije commented Dec 9, 2018

@xPaw, thoughts? https://deploy-preview-175--thelounge.netlify.com/robots.txt correctly shows Disallow: /, and once merged the master version should show Allow: /. 🤷‍♂️

@xPaw
Copy link
Member Author

xPaw commented Dec 9, 2018

Looks good

@xPaw xPaw merged commit 4da6061 into master Dec 23, 2018
@xPaw xPaw deleted the xpaw/robots branch December 23, 2018 19:06
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.

None yet

2 participants