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 asset manifest check and warning #1063

Merged
merged 2 commits into from
May 19, 2021
Merged

Remove asset manifest check and warning #1063

merged 2 commits into from
May 19, 2021

Conversation

bakerkretzmar
Copy link
Contributor

@bakerkretzmar bakerkretzmar commented May 19, 2021

This PR reverts #729, removing the checks to determine at runtime if Telescope's assets are published and up to date.

The exception that is thrown if the assets appear to be missing doesn't play nicely with Vapor because it assumes that the assets should be present on the local filesystem at runtime, and the warning feels unnecessary too since for quite a long time now the docs have had very clear instructions about how to keep assets up to date automatically.

Closes #1062.

See also #594 and laravel/horizon#554, this warning wasn't immediately popular when it was added and both Telescope and Horizon now provide documentation that make it unnecessary.

@taylorotwell taylorotwell merged commit 87f40db into laravel:4.x May 19, 2021
@bakerkretzmar bakerkretzmar deleted the remove-asset-check branch May 19, 2021 15:40
@barryvdh
Copy link
Contributor

Can't we just remove the exception if no file is found, instead of not checking at all? If the version is mismatch it's easy to fix so don't really see what the problem is (outside of the exception).

@driesvints
Copy link
Member

@barryvdh Probably best that you propose something with a PR

@bakerkretzmar
Copy link
Contributor Author

Vapor UI does this in a really cool way actually, with a middleware: https://github.com/laravel/vapor-ui/blob/master/config/vapor-ui.php. That strikes me as the best of both worlds, basically makes it completely configurable by the user.

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.

'Assets not published' exception thrown with default Vapor setup
4 participants