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

Remove packages.path config and replace it with public_dir #118

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Sep 26, 2019

The only thing needed to start the server is having it pointed to the public directory to serve files from there. What the internal name is for the packages directory is an implementation detail.

Some refactoring of the code was needed for this change. Now path variables are passed to the handler instead of using a global variable. This also makes it simpler to test individually instead of modifying global variables.

Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

The only thing needed to start the server is having it pointed to the public directory to serve files from there. What the internal name is for the packages directory is an implementation detail.

Some refactoring of the code was needed for this change. Now path variables are passed to the handler instead of using a global variable. This also makes it simpler to test individually instead of modifying global variables.
@ruflin ruflin force-pushed the public_dir-config branch from 7c9510f to 0f63bc7 Compare October 21, 2019 12:36
@ruflin
Copy link
Contributor Author

ruflin commented Oct 21, 2019

Rebase was needed as this PR confliced with the cacheTime PR.

@ruflin ruflin merged commit b63b48a into elastic:master Oct 21, 2019
@ruflin ruflin deleted the public_dir-config branch October 21, 2019 12:50
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