-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
[2.x] Rewrite the Asset API #1904
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x-dev #1904 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 1876 1884 +8
===========================================
Files 192 193 +1
Lines 5011 5028 +17
===========================================
+ Hits 5011 5028 +17 ☔ View full report in Codecov by Sentry. |
See #1904 for why we are skipping the normal deprecation process.
See #1904 for why we are skipping the normal deprecation process.
e621f54
to
9ba410a
Compare
We'll see if we name this mediaFiles or assets
@@ -3,15 +3,15 @@ | |||
|
|||
{{-- The compiled Tailwind/App styles --}} | |||
@if(config('hyde.load_app_styles_from_cdn', false)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also have a wrapper like HydeFront::enabled()
a818a0e
to
532e35f
Compare
This option is best because it: 1. Maintains consistency with the rest of the configuration file. 2. Aligns with the explanatory comment. 3. Is clear and concise. It is part of the HydePHP v2 Asset API refactors #1904
…ting Rename config option `hyde.enable_cache_busting` to `hyde.cache_busting`
720f3e0
to
c936cbe
Compare
43853d4
to
84672f1
Compare
Let's break down the changes in this Pull Request (PR) and ensure the release notes and upgrade guide accurately reflect them. Key Changes and Affected Files
Release Notes and Upgrade Guide Analysis
Overall Assessment Based on the information provided, the release notes and upgrade guide appear to be fully implemented and accurate in relation to the PR's changes. They comprehensively cover the key aspects of the update and provide clear guidance for users to upgrade their sites. |
Abstract
Background
The Problem: The current system is confusing and unintuitive with multiple and inconsistent ways to access assets.
The Cause: This is due to multiple code locations have had overlapping code independently implemented, without thinking about the bigger picture.
The Solution: This PR rewrites the web of related code into a unified Asset API, designed to provide a consistent interface to interact with the project's media asset.
Scope
We create the new solution by thinking about how users will actually use these features, which can be summarized into the following scope:
Podcast
Overwhelmed by this massive changelog? Listen to this AI-generated podcast episode by NotebookLM by Google which was fed the commit logs. Improved version available here
output.mp4
Roadmap
asset()
that does not existmediaLink
helper as it can be merged into the mainasset()
helpergetContentLength()
togetLength()
Development
preferQualifiedUrl
parameter from the asset helper #1913mediaLink
method #1914Asset::hasMediaFile
toAsset::exists
#1957hyde.enable_cache_busting
tohyde.cache_busting
#1980I don't think we'll be doing a v1 tie in here (normally we would backport the new feature and deprecate the old code to provide a migration path, but considering that v1 is in feature freeze #1877 I don't think this complexity would provide value, as there is too little of a time frame between when the next minor release comes out and the v2 release, so it's easier to just migrate to v2 directly)
Hyde::mediaLink
in favour ofHyde::asset
in v1. [1.x] DeprecateHyde::mediaLink
being replaced byHyde::asset()
in v2 #1993