Skip to content

[Notifier] [GoogleChat] [BC BREAK] Rename threadKey parameter to thread_key + set parameter via ctor #39579

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
Merged
merged 1 commit into from
Jan 15, 2021
Merged

[Notifier] [GoogleChat] [BC BREAK] Rename threadKey parameter to thread_key + set parameter via ctor #39579

merged 1 commit into from
Jan 15, 2021

Conversation

OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Dec 20, 2020 •
edited
Loading

Q A
Branch? 5.x, but BC BREAK for experimental bridge
Bug fix? no
New feature? no
Deprecations? no
Tickets ---
License MIT
Doc PR symfony/symfony-docs#14834
Recipe symfony/recipes#871

All bridges receive their options via the constructor and use snake_case parameters.

Todos

  • Update recipe
  • Update documentation

cc @GromNaN as you provided the bridge

@OskarStark OskarStark closed this Dec 21, 2020
@OskarStark OskarStark reopened this Dec 21, 2020
@OskarStark OskarStark force-pushed the threadKey branch 2 times, most recently from 6f3cbe7 to 8cfb2ed Compare December 22, 2020 19:51
@nicolas-grekas nicolas-grekas added this to the 5.x milestone Dec 23, 2020
@OskarStark OskarStark force-pushed the threadKey branch 3 times, most recently from ceda482 to eb4f048 Compare December 29, 2020 08:18
@OskarStark OskarStark force-pushed the threadKey branch 2 times, most recently from 15d5d16 to 486109a Compare January 6, 2021 14:42
@OskarStark
Copy link
Contributor Author

@fabpot would you please give me a review here? @GromNaN, who introduced the bridge is ok with this change.

Thanks

This was referenced Jan 15, 2021
Merged
Merged
jderusse
jderusse reviewed Jan 15, 2021
View reviewed changes
src/Symfony/Component/Notifier/Bridge/GoogleChat/Tests/GoogleChatTransportFactoryTest.php
@@ -47,6 +47,7 @@ public function supportsProvider(): iterable
public function incompleteDsnProvider(): iterable
{
yield 'missing credentials' => ['googlechat://chat.googleapis.com/v1/spaces/AAAAA_YYYYY/messages'];
Copy link
Member

@jderusse jderusse Jan 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think this test is wrong, as our DSN should not contains v1/spaces the exception might not be related to the missing credentials

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should then be fixed in 5.2 branch, but I think you are right 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a PR: #39848

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR merged and this one rebase @jderusse

@OskarStark OskarStark merged commit 3796af6 into symfony:5.x Jan 15, 2021
OskarStark added a commit to symfony/symfony-docs that referenced this pull request Jan 15, 2021
@OskarStark
feature #14834 [Notifier] Rename threadKey to thread_key (OskarStark)
ba51253 
This PR was merged into the 5.3-dev branch.

Discussion
----------

[Notifier] Rename threadKey to thread_key

Follows symfony/symfony#39579

cc @GromNaN

Commits
-------

debedc3 Rename threadKey to thread_key
@fabpot fabpot mentioned this pull request Apr 18, 2021
Merged
@IonBazan IonBazan mentioned this pull request Sep 21, 2021
Merged
fabpot added a commit that referenced this pull request Sep 21, 2021
@fabpot
feature #43121 [Notifier] [GoogleChat] remove support for deprecated …
4ae7652 
…"threadKey" parameter (IonBazan)

This PR was merged into the 5.4 branch.

Discussion
----------

[Notifier] [GoogleChat] remove support for deprecated "threadKey" parameter

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

Removing the deprecation error introduced when renaming `threadKey` to `thread_key` in #39579

Commits
-------

aada697 remove support for deprecated "threadKey" parameter
@matks matks mentioned this pull request Oct 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@jderusse jderusse jderusse approved these changes

Assignees
No one assigned
Labels
BC Break Notifier Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
5 participants
@OskarStark @fabpot @jderusse @nicolas-grekas @carsonbot