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.

Sign up for GitHub

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

Jump to bottom

Issue #2391251 by Pol, Fabianx: Create a messenger service based on https://www.drupal.org/node/2278383 #23

Merged

LionsAd merged 26 commits into LionsAd:7.x-1.x from drupol:Implements-Messenger-service

Jun 5, 2015

Collaborator

drupol commented Jun 3, 2015

This is just a test that I'm doing, I don't know where this is gonna lead.

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling abe0664 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

Collaborator

dawehner commented Jun 3, 2015

I thought the idea on the longrun would be to use \Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface, so we could implement that interface already?

Collaborator Author

drupol commented Jun 3, 2015

I'll check this out.

Collaborator Author

drupol commented Jun 3, 2015

How to use it with the MessengerInterface ?

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling e21233c on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

2 similar comments

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling e21233c on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling e21233c on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling e21233c on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling e21233c on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

coveralls commented Jun 3, 2015

Coverage remained the same at 100.0% when pulling 47c126a on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

LionsAd reviewed

View reviewed changes

.idea/misc.xml

+              <?xml version="1.0" encoding="UTF-8"?>
+              <project version="4">
+                <component name="ProjectRootManager" version="2" />
+              </project>

Owner

LionsAd Jun 4, 2015

What is that?

Collaborator Author

drupol Jun 4, 2015

Oops sorry, it comes from PHPStorm.

Owner

LionsAd commented Jun 4, 2015

I think for now the MessengerInterface is fine.

As it is not part of Drupal 8, I think the best would be to move the MessengerInterface to service_container as well for now.

I'll ping alexpott if Core is interested in the LegacyMessenger here.

As always, we'll need some integration tests.

LionsAd added Needs review Needs tests labels

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 9899d69 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

LionsAd reviewed

View reviewed changes

src/Messenger/LegacyMessenger.php

+               * @codeCoverageIgnore
+               */
+              class LegacyMessenger implements MessengerInterface {
+                public function addMessage($message, $type = self::STATUS, $repeat = FALSE) {

Owner

LionsAd Jun 4, 2015

Missing {@inheritdoc}

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 28ba224 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

LionsAd reviewed

View reviewed changes

lib/Drupal/service_container/Messenger/MessengerInterface.php

		@@ -0,0 +1,81 @@
		<?php

Owner

LionsAd Jun 4, 2015

I think moving to src/Messenger is better.

Tests had only been in lib/Drupal/service_container, because simpletest in D7 was missing PSR-4 support. It has that now, so for newer versions of Drupal we can switch over.

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 58aa868 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

1 similar comment

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 58aa868 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 58aa868 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

LionsAd added RTBM and removed Needs review labels

Owner

LionsAd commented Jun 4, 2015

RTBM, just needs some tests.

Collaborator

dawehner commented Jun 4, 2015

Hey, getting this in maybe helps people in the core issue to be convinced :P

coveralls commented Jun 4, 2015

Coverage remained the same at 100.0% when pulling 0a77022 on Polzme:Implements-Messenger-service into 1d74456 on LionsAd:7.x-1.x.

Pol Dellaiera and others added 22 commits

June 5, 2015 13:55


          Throw proper error when trying to access methods that are not available.

acb352d


          Remove PHPStorm files.

95b9a6b


          Move MessengerInterface in service_container.

f6cd48a


          Add missing @inheritdoc

146f7c9


          Move MessengerInterface to the right place.

eee4e17


          First essay of a test.

552e4a9


          Fix file description.

5080c18


          Add tests.

019a4f1


          Update code according to comments.

e275c32


          Update code according to comments.

7d262d5


          This should fix tests.

87fc7c3


          Now using the new Drupal7 service.


          Add unit test. Some of them are still missing.

aafef1e


          Remove @CodeCoverageIgnore.

57553ad


          Fix name of the method to cover.

b473bec


          Tests update.

abfb11c


          Fix interface and add data provider.

a552cfc


          Fix remaining issues.

d187e2e


          Add @dataProvider for addMessage method coverage.

e7bf354


          Update naming.

e535c4a


          Add missing doxygen block.

9d57788


          Add missing doxygen block.

75dc6f8

LionsAd removed the Needs rebase label

LionsAd changed the title ~~Implements Messenger service.~~ Issue #2391251 by Pol, Fabianx: Create a messenger service based on https://www.drupal.org/node/2278383

coveralls commented Jun 5, 2015

Coverage remained the same at 100.0% when pulling 9d57788 on Polzme:Implements-Messenger-service into 73f9e47 on LionsAd:7.x-1.x.

coveralls commented Jun 5, 2015

Coverage remained the same at 100.0% when pulling 75dc6f8 on Polzme:Implements-Messenger-service into 73f9e47 on LionsAd:7.x-1.x.

coveralls commented Jun 5, 2015

Coverage remained the same at 100.0% when pulling 75dc6f8 on Polzme:Implements-Messenger-service into 73f9e47 on LionsAd:7.x-1.x.

LionsAd added a commit that referenced this pull request


          Merge pull request #23 from Polzme/Implements-Messenger-service

1428c46

Issue #2391251 by Pol, Fabianx: Create a messenger service based on https://www.drupal.org/node/2278383

LionsAd merged commit 1428c46 into LionsAd:7.x-1.x

drupol deleted the Implements-Messenger-service branch

June 8, 2015 17:51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment