Is there a desire to integrate with media?

I created a basic module for media integration here:
https://github.com/WondrousLLC/rh_media

Perhaps we could add this as a submodule? Patch is attached.

CommentFileSizeAuthor
#6 interdiff-2841865-1-6.txt1.05 KBtassilogroeper
#6 integration_with_media-2841865-6.patch1.56 KBtassilogroeper
add_media_submodule.patch1.56 KByobottehg

Comments

yobottehg created an issue. See original summary.

yobottehg’s picture

yobottehg commented
Assigned: yobottehg » Unassigned
Status: Active » Needs review
berdyshev’s picture

berdyshev commented

Works nice.
I don't see any issues with code.
One thing: maybe it's better to call module rh_media_entity since the entity is defined in this module, but media module is a "glue" for the whole media ecosystem.

yobottehg’s picture

yobottehg commented

Thats true.

I just sticked to the naming of the other submodules like rh_file for example which works only for file_entity.
Perhaps someone other has a opinion on this?

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs review » Needs work

Small nitpicks. :)

  1. +++ b/modules/rh_media/src/Plugin/RabbitHoleEntityPlugin/Media.php
@@ -0,0 +1,18 @@
+ *  id = "rh_media",
+ *  label = @Translation("Media"),
+ *  entityType = "media"

    One space missing before id, label and entityType, should be 3. 

  2. +++ b/modules/rh_media/src/Plugin/RabbitHoleEntityPlugin/Media.php
@@ -0,0 +1,18 @@
+class Media extends RabbitHoleEntityPluginBase {
+
+}

    If empty, could be { }. 

+++ b/modules/rh_media/rh_media.info.yml
--- /dev/null
+++ b/modules/rh_media/rh_media.module

+++ b/modules/rh_media/rh_media.module
@@ -0,0 +1,18 @@
+ * Contains rh_file.module.

rh_file or rh_media? :)

tassilogroeper’s picture

tassilogroeper commented
StatusFileSize
new integration_with_media-2841865-6.patch1.56 KB
new interdiff-2841865-1-6.txt1.05 KB

thank you @ yongt9412. fixed the wording and the indentation.

tassilogroeper’s picture

tassilogroeper commented
Status: Needs work » Needs review
Anonymous’s picture

Anonymous (not verified) commented

Works perfectly for me.

berdyshev’s picture

berdyshev commented
Status: Needs review » Patch (to be ported)
berdyshev’s picture

berdyshev commented
Status: Patch (to be ported) » Reviewed & tested by the community

dylan donkersgoed’s picture

dylan donkersgoed commented
Status: Reviewed & tested by the community » Fixed

The code looks good and seems to function well. I think it makes sense to bring this in as a rabbit hole submodule for D8. I've merged it in and tagged a new release including it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.