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

Update to match with Laravel 5.7 #103

Closed
wants to merge 2 commits into from
Closed

Update to match with Laravel 5.7 #103

wants to merge 2 commits into from

Conversation

incoming-th
Copy link
Contributor

Changing some value in the lines from Laravel to reflect the newest 5.7 version. Also added some instructions to make it clearer to follow.

Changing some value in the lines from Laravel to reflect the newest 5.7 version. Also added some instructions to make it clearer to follow.
docs/Laravel.md Outdated Show resolved Hide resolved
docs/Laravel.md Show resolved Hide resolved
docs/Laravel.md Show resolved Hide resolved
@mnapoli
Copy link
Member

mnapoli commented Dec 4, 2018

The build is failing on master so this is not your PR (if anyone is up to fixing it BTW) so don't worry about it.

Update to reflect the new ENV variables used by Laravel 5.7 such as APP_BASE_PATH and VIEW_COMPILED_PATH.
Copy link
Contributor Author

@incoming-th incoming-th left a comment

Choose a reason for hiding this comment

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

This make use of the new ENV variable and replace the previous changes.

docs/Laravel.md Outdated Show resolved Hide resolved
- 'compiled' => realpath(storage_path('framework/views')),
- 'compiled' => env(
- 'VIEW_COMPILED_PATH',
- realpath(storage_path('framework/views'))
+ 'compiled' => storage_path('framework/views'),
Copy link
Member

Choose a reason for hiding this comment

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

I believe we can now remove this whole bit because of the VIEW_COMPILED_PATH variable in .env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are correct, sorry for this was using online editor. I will review all in one shot and submit a better version.

# We cannot store sessions to disk: if you don't need sessions (API, etc.) use `array`, else store sessions in database
SESSION_DRIVER=array
# Logging to stderr allows the logs to end up in Cloudwatch
LOG_CHANNEL=stderr
# This allows to generate relative file paths for the lambda
# This allows to generate relative file paths for the lambda because when generating the optimized config absolute paths will be everywhere and they will not work because your machine and the lambda environment do not match.
# To avoid that problem, set APP_BASE_PATH variable will allow us to replace the absolute path by `.` (relative path) when generating the lambda.
APP_DIR=.
Copy link
Member

Choose a reason for hiding this comment

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

APP_DIR should be replaced by the official APP_BASE_PATH variable now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct again my mistake.

@mnapoli
Copy link
Member

mnapoli commented Dec 4, 2018

This is great that means less things to override!

@incoming-th
Copy link
Contributor Author

incoming-th commented Dec 4, 2018

This is great that means less things to override!

I made a new PR with a better version #104 , you can reject this one #103 Thank you for your review.

@mnapoli
Copy link
Member

mnapoli commented Dec 4, 2018

👍

@mnapoli mnapoli closed this Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants