CommentFileSizeAuthor
#13 drupal-1969692-13.patch10.53 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,554 pass(es). View
#13 interdiff.txt529 bytesdawehner
#12 drupal-1969692-12.patch10.53 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
#12 interdiff.txt2.14 KBdawehner
#8 drupal-1969692-8.patch10.5 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View
#7 drupal-1969692-7.patch10.5 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 56,131 pass(es). View
#5 drupal-1969692-5.patch10.46 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 54,092 pass(es), 102 fail(s), and 5 exception(s). View
#1 drupal-1969692-1.patch10.61 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 54,298 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

dawehner’s picture

Status: Active » Needs review
Issue tags: +Plugin system, +Annotation
FileSize
10.61 KB
PASSED: [[SimpleTest]]: [MySQL] 54,298 pass(es). View

There we go

ParisLiakos’s picture

Status: Reviewed & tested by the community » Needs review

Cool, thanks:)
That means we can get rid of

$info = $this->getDefinition();
..
$form['processors'][$info['id']] = array();

for $this->id?

in \Drupal\aggregator\Plugin\aggregator\processor\DefaultProcessor::settingsForm()

EDIT: nvm, this is totally wrong:)

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

its good to go:)

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorFetcher.phpundefined
@@ -0,0 +1,43 @@
+namespace Drupal\aggregator\Plugin\Annotation;

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorParser.phpundefined
@@ -0,0 +1,43 @@
+namespace Drupal\aggregator\Plugin\Annotation;

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorProcessor.phpundefined
@@ -0,0 +1,43 @@
+namespace Drupal\aggregator\Plugin\Annotation;

While we're only using them for plugins, Annotations are technically unrelated to that subsystem (yeah, seriously), and should just be in their own namespace: \Drupal\$module\Annotation

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorFetcher.phpundefined
@@ -0,0 +1,43 @@
+   * The string should be wrapped in a @Translatable()
...
+   * The string should be wrapped in a @Translatable()

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorParser.phpundefined
@@ -0,0 +1,43 @@
+   * The string should be wrapped in a @Translatable()
...
+   * The string should be wrapped in a @Translatable()

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/Annotation/AggregatorProcessor.phpundefined
@@ -0,0 +1,43 @@
+   * The string should be wrapped in a @Translatable()
...
+   * The string should be wrapped in a @Translatable()

I'm not sure that this is necessary to repeat for every property. But if it is, we should probably just do @see \Drupal\Core\Annotation\Translatable or something similar.

dawehner’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
10.46 KB
FAILED: [[SimpleTest]]: [MySQL] 54,092 pass(es), 102 fail(s), and 5 exception(s). View

Status: Needs review » Needs work

The last submitted patch, drupal-1969692-5.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
10.5 KB
PASSED: [[SimpleTest]]: [MySQL] 56,131 pass(es). View

Rerolled. I guess this should work also this time.

dawehner’s picture

FileSize
10.5 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Rerole.

Status: Needs review » Needs work
Issue tags: -Plugin system, -Annotation

The last submitted patch, drupal-1969692-8.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +Plugin system, +Annotation

#8: drupal-1969692-8.patch queued for re-testing.

amateescu’s picture

Status: Needs review » Needs work

I could only find some small nitpicks here, otherwise looks great.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Annotation/AggregatorFetcher.php
@@ -0,0 +1,43 @@
+ * Contains \Drupal\aggregator\Annotation\AggregatorFetcher.
+ */
+namespace Drupal\aggregator\Annotation;

Missing empty line.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Annotation/AggregatorFetcher.php
@@ -0,0 +1,43 @@
+}
+

Extra empty line at the end of the file?

The two above apply to all new class files, not just AggregatorFetcher.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Annotation/AggregatorFetcher.php
@@ -0,0 +1,43 @@
+   * @ingroup plugin_translatable
+   *
+   * @var \Drupal\Core\Annotation\Translation

@ingroup should be below @var.

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.14 KB
10.53 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Thanks!

dawehner’s picture

FileSize
529 bytes
10.53 KB
PASSED: [[SimpleTest]]: [MySQL] 55,554 pass(es). View

One missing newline.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

All good now.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

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