Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

PLAT-629 Fix ESU queues #658

Merged
merged 3 commits into from
Nov 28, 2016
Merged

PLAT-629 Fix ESU queues #658

merged 3 commits into from
Nov 28, 2016

Conversation

Saphyel
Copy link
Contributor

@Saphyel Saphyel commented Nov 25, 2016

@Saphyel Saphyel added this to the 1.20.0 milestone Nov 25, 2016
bimsonz
bimsonz suggested changes Nov 28, 2016
View reviewed changes
Copy link
Contributor

@bimsonz bimsonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tested this in RND ? can we do this before its merged ? Also, one comment about camel case!

profiles/cr/modules/custom/cr_email_signup/src/MessageQueue/Sender.php
* @param array $data
*/
public function sendTo($name, $data){
$this->sendQmessage($name, $this->fillmessage($data));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using camel case elsewhere we should be consistent and use it here as well!

Copy link
Contributor Author

@Saphyel Saphyel Nov 28, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far I know functions are camelcase and variables are with _ or did we change that?

Copy link
Contributor

@bimsonz bimsonz Nov 28, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saphyel fillmessage() not a method then ?

@Saphyel
Copy link
Contributor Author

Saphyel commented Nov 28, 2016

Has been tested in RND17 (is still deployed in test), you can check the PR in that repo & also you can ask to Corin

@bimsonz bimsonz merged commit 9957c2e into develop Nov 28, 2016
@bimsonz bimsonz deleted the PLAT-629_ESU branch November 28, 2016 15:21
@Saphyel Saphyel mentioned this pull request Dec 5, 2016
Merged
@pvhee pvhee mentioned this pull request Jan 5, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bimsonz bimsonz bimsonz approved these changes

Assignees
No one assigned
Labels
Status: Needs Review Status: Reviewed Status: Waiting feedback
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Saphyel @bimsonz