Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add setTopicArn methods to SnsContext and SnsQsContext #1189

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.

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

Merged
makasim merged 1 commit into php-enqueue:master from gdsmith:allow-sns-topic-setting
Aug 22, 2021

Conversation

Copy link
Contributor

@gdsmith gdsmith commented Jul 30, 2021

Enables us to prevent #1188

@gdsmith gdsmith force-pushed the allow-sns-topic-setting branch from 690f8ec to 425d5e0 Compare August 18, 2021 14:10
SnsContext::class,
SnsContext::class
));
throw new \InvalidArgumentException(sprintf('The $snsContext argument must be either %s or callable that returns %s once called.', SnsContext::class, SnsContext::class));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any particular reason for these formatting changes? IMO they reduce readability.

Copy link
Contributor Author

gdsmith commented Aug 19, 2021 via email

No. Think that must have happened as part of the rebase.
...
On 2021年8月19日, 14:53 Andrew, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkg/snsqs/SnsQsContext.php <#1189 (comment)> : > @@ -53,23 +53,15 @@ public function __construct($snsContext, $sqsContext) } elseif (is_callable($snsContext)) { $this->snsContextFactory = $snsContext; } else { - throw new \InvalidArgumentException(sprintf( - 'The $snsContext argument must be either %s or callable that returns %s once called.', - SnsContext::class, - SnsContext::class - )); + throw new \InvalidArgumentException(sprintf('The $snsContext argument must be either %s or callable that returns %s once called.', SnsContext::class, SnsContext::class)); Is there any particular reason for these formatting changes? IMO they reduce readability. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1189 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG53F67TNXSZHMCCT5OCKDT5TPCDANCNFSM5BH3I3GA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

Copy link
Contributor

Pretty sure I saw them before the rebase as well.

Copy link
Contributor Author

gdsmith commented Aug 20, 2021

Changes are from composer cs-fix. I can revert them if you'd prefer

Copy link
Contributor Author

gdsmith commented Aug 20, 2021

Actually, seems like this would fail the build if I revert those changes:

./bin/php-cs-fixer fix --config=.php_cs.php --no-interaction --dry-run --diff -v --path-mode=intersection -- 'pkg/sns/SnsContext.php' 'pkg/snsqs/SnsQsContext.php'
PHP CS Fixer 2.18.6 Remote Void by Fabien Potencier and Dariusz Ruminski
Runtime: PHP 7.3.11
Loaded config default from ".php_cs.php".
Using cache file "/Volumes/Mukuru/taurus/third-party/enqueue-dev/var/.php_cs.cache".
SF
Legend: ?-unknown, I-invalid file syntax (file ignored), S-skipped (cached or empty file), .-no changes, F-fixed, E-error
 1) /Volumes/Mukuru/taurus/third-party/enqueue-dev/pkg/snsqs/SnsQsContext.php (single_line_throw)
 ---------- begin diff ----------
--- Original
+++ New
@@ @@
 } else {
- throw new \InvalidArgumentException(sprintf(
- 'The $snsContext argument must be either %s or callable that returns %s once called.',
- SnsContext::class,
- SnsContext::class)
- );
+ throw new \InvalidArgumentException(sprintf('The $snsContext argument must be either %s or callable that returns %s once called.', SnsContext::class, SnsContext::class));
@@ @@
 } else {
- throw new \InvalidArgumentException(sprintf(
- 'The $sqsContext argument must be either %s or callable that returns %s once called.',
- SqsContext::class,
- SqsContext::class)
- );
+ throw new \InvalidArgumentException(sprintf('The $sqsContext argument must be either %s or callable that returns %s once called.', SqsContext::class, SqsContext::class));
 }

Copy link
Contributor

Wow ok :D

Copy link
Contributor Author

gdsmith commented Aug 20, 2021

So is this good for deploy and release now 🤞

@makasim makasim merged commit 14f080a into php-enqueue:master Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@andrewmy andrewmy andrewmy left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /