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

fix: do not hardcode siteConfig path #1150

Merged
merged 6 commits into from
Dec 7, 2018

Conversation

JoelMarcey
Copy link
Contributor

@JoelMarcey JoelMarcey commented Dec 7, 2018

Motivation

Fix broken build

Fixes #1149

Have you read the Contributing Guidelines on pull requests?

Yes, I helped write them.

Test Plan

Local Testing

Related PRs

#1143

Do not hardcode path in require to siteConfig
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Dec 7, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 4e42759

https://deploy-preview-1150--docusaurus-preview.netlify.com

I think the test may be wrong too.
@JoelMarcey
Copy link
Contributor Author

JoelMarcey commented Dec 7, 2018

@endiliey, @yangshun -- the v1 tests are failing, but I am wondering if the test itself is not right due to the faulty include in the first place.

@JoelMarcey
Copy link
Contributor Author

Unless I am totally missing something here, this should be the proper fix to the path issue.

@endiliey
Copy link
Contributor

endiliey commented Dec 7, 2018

This is because on test. process.cwd() is actually not website, it's the root project directory. That's why I dislike having process.cwd() on v1. We have to mock it during the test.

@endiliey
Copy link
Contributor

endiliey commented Dec 7, 2018

I'm gonna patch this PR and make server/utils to be purely functional without side effect. Having require siteConfig made the utils functions having side effect.

@JoelMarcey
Copy link
Contributor Author

@endiliey - I trust you 👍

@endiliey endiliey changed the title Do not hardcode path in require to siteConfig fix: do not hardcoded siteConfig path Dec 7, 2018
@endiliey endiliey changed the title fix: do not hardcoded siteConfig path fix: do not hardcode siteConfig path Dec 7, 2018
@endiliey endiliey merged commit c78a8b4 into facebook:master Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants