Code style cleanup for media module files [#1887576]

The attached patches are from a coder review. There are still some issues that coder is reporting- I chose to ignore a few that seemed low priority for the moment. I broke this patch up into individual patches for the sheer size of it. It is probably the case that some of these will need to be broken off into their own issues- I'm not sure what the usual protocol is around handling this number of changes.

Comment	File	Size	Author
#3	streamwrapper.diff	16.33 KB	arthurf
#3
#3	media.browser.inc_.diff	6.42 KB	arthurf
#3
#3	media.filter.inc_.diff	7.77 KB	arthurf
#3
#3	media.xml_.inc_.diff	2.15 KB	arthurf
#3
#3	monolithic.diff	80.44 KB	arthurf
#3
#1	coder_monlithic.diff	80.25 KB	arthurf
#1
	MediaReadOnlyStreamWrapper.inc_.diff	16.14 KB	arthurf

	mediafield.install.diff	613 bytes	arthurf

	MediaEntityTranslationHandler.inc_.diff	669 bytes	arthurf

	MediaBrowserPluginInterface.inc_.diff	1.24 KB	arthurf

	MediaBrowserPlugin.inc_.diff	1.32 KB	arthurf

	media.xml_.inc_.diff	2.15 KB	arthurf

	media.views_.inc_.diff	2.6 KB	arthurf

	media.variables.inc_.diff	6.66 KB	arthurf

	media.theme_.inc_.diff	2.69 KB	arthurf

	media.test.diff	425 bytes	arthurf

	media.pages_.inc_.diff	917 bytes	arthurf

	media.module.diff	17.15 KB	arthurf

	media.media_.inc_.diff	1007 bytes	arthurf

	media.install.diff	3.23 KB	arthurf

	media.inc_.diff	1.19 KB	arthurf

	media.filter.inc_.diff	7.77 KB	arthurf

	media.fields.inc_.diff	1.01 KB	arthurf

	media.entity.test.diff	1.01 KB	arthurf

	media.browser.inc_.diff	6.42 KB	arthurf

	media.admin_.inc_.diff	3.38 KB	arthurf

	media_views_plugin_style_media_browser.inc_.diff	1.41 KB	arthurf

	media_internet.media_.inc_.diff	538 bytes	arthurf

	media_internet.api_.php_.diff	834 bytes	arthurf

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

arthurf CreditAttribution: arthurf commented 13 January 2013 at 17:25

File	Size
coder_monlithic.diff	80.25 KB

Here's the monolithic version of all the above patches

Comment #2

ParisLiakos CreditAttribution: ParisLiakos commented 13 January 2013 at 19:22

Status:

Needs review

» Needs work

Awesome! thanks a lot:)
Just a few issues i found

+++ b/includes/MediaReadOnlyStreamWrapper.incundefined
@@ -470,23 +429,46 @@ abstract class MediaReadOnlyStreamWrapper implements DrupalStreamWrapperInterfac
+   * Implements unlink.

Should be
Implements DrupalStreamWrapperInterface::unlink()

+++ b/includes/MediaReadOnlyStreamWrapper.incundefined
@@ -470,23 +429,46 @@ abstract class MediaReadOnlyStreamWrapper implements DrupalStreamWrapperInterfac
+   * Implements rename.

Same for the rest

+++ b/includes/media.browser.incundefined
@@ -214,10 +234,14 @@ function media_attach_browser_js(&$element) {
+    'browserUrl' => url('media/browser', array(
+      'query' => array('render' => 'media-popup'),
+      )
+    ),
+    'styleSelectorUrl' => url('media/-media_id-/format-form', array(
+      'query' => array('render' => 'media-popup'),
+      )
+    ),

Seems you missed that:) indentation is a bit off

+++ b/includes/media.filter.incundefined
@@ -499,13 +506,13 @@ function media_filter_invalidate_caches($fid = FALSE) {
+ * @param ing $fid

should be int:)

+++ b/includes/media.xml.incundefined
@@ -1,23 +1,24 @@
+ * @return objectl

i guess 1 is typo?

Comment #3

arthurf CreditAttribution: arthurf commented 14 January 2013 at 01:33

File	Size
monolithic.diff	80.44 KB

media.xml_.inc_.diff	2.15 KB

media.filter.inc_.diff	7.77 KB

media.browser.inc_.diff	6.42 KB

streamwrapper.diff	16.33 KB

Here are the modified individual patches and the monolithic. I'm not sure what coder wants for that array structure in media.browser.inc- it wasn't flagging that before, but I indented that further and it doesn't complain.