Problem/Motivation
The Views RSS: Media module currently has a dependency on the Libraries module to load getID3. However, getID3 can be loaded through the composer class loader instead.
Furthermore, getID3 provides additional metadata for multimedia files that might not be necessary for all installations. I suggest we implement a submodule views_rss_media_getid3 that provides getID3 integration.
Proposed resolution
Remove the dependency on Libraries module and getID3, use class loader instead. Provide getID3 integration through a new views_rss_media_getid3 submodule.
Remaining tasks
- Write a patch
- Review
- Commit
- Document how to use the composer class loader to load getID3
- Update Drupal.org documentation to reflect the new method for loading getID3
User interface changes
The getID3 library used to provide additional metadata for multimedia file formats is now installed through composer. Execute the following command to add this library to your installation:
composer require james-heinrich/getid3
API changes
- views_rss_media no longer depends on getID3.
- views_rss_media no longer depends on the Libraries module.
- getID3 integration is now provided through a new
views_rss_media_getid3submodule.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3068934-9.patch | 17.42 KB | idebr |
Comments
Comment #2
idebr commentedAttached patch removes the dependency on Libraries module and getID3, and uses the class loader instead. It provides getID3 integration through a new views_rss_media_getid3 submodule.
Comment #3
idebr commentedFixed two occurrences where an URL attribute referred to a Drupal URI instead of a public URL, and a copy/paste error.
Comment #4
lendudeThis makes the new module compatible with D9.
Comment #5
idebr commentedAttached patch replaces methods that were removed in Drupal 9.
Comment #6
idebr commentedAttached patch fixed the file URL generation.
Comment #7
lendudeReviewed the code and tested locally for client and works great.
Comment #8
gisleThe patch does not apply cleanly to the latest dev-snapshot (it needs to be rerolled becuse of changes to
views_rss_media.install). The rest apply.However, enabling the new
views_rss_media_getid3submodule fails with the message:I am fine with proposed solution (remove the dependency on Libraries module and getID3, use composerclass loader instead, provide getID3 integration through a new
views_rss_media_getid3submodule). However, there seems to be some bits missing. To use composer to load an external library, I belive the library needs to be declared as a requirement in the module'scomposer.json, and there is none.Also, the documention for the module needs to be updated to reflect the altered method for getting the library. Don't go ahead and edit the documentation wiki on Drupal.org, instead add it to the issue summary, in the section "User interface changes". The new documentation will be reviewed along with the code changes.
Comment #9
idebr commented#8 A Drupal submodule cannot list its own requirements, for example in views_rss_media_getid3/composer.json so you either have add a requirement in the root of the repository or install it separately.
Attached patch is a reroll against the latest dev
I also updated the 'User interface changes' in the issue summary with the new installation instructions for the getID3 library.
Comment #11
gisleReviewed and found to be ready. Added a README.md with instructions about installing the submodule.
Pushed to the latest snapshot of 8.x-1.x-dev. Thanks for the patch.