Skip to content

[Notifier] Add telegram tests #34539

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
merged 1 commit into from
Nov 30, 2019
Merged

[Notifier] Add telegram tests #34539

merged 1 commit into from
Nov 30, 2019

Conversation

Jeroeny
Copy link
Contributor

@Jeroeny Jeroeny commented Nov 23, 2019

Q A
Branch? 5.0
Bug fix? no
New feature? no
Deprecations? no
Tickets See #33687
License MIT

Adds tests for the Telegram bridge of the Notifier component. Not sure if this is the right way to go, but would love some feedback.

#SymfonyHackday

@@ -20,6 +20,9 @@
"symfony/http-client": "^4.3|^5.0",
"symfony/notifier": "~5.0.0"
},
"require-dev": {
"symfony/event-dispatcher": "^4.3|^5.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added because the AbstractTransport uses it in it's constructor. Not sure if it should actually be in require?

@nicolas-grekas nicolas-grekas added this to the next milestone Nov 24, 2019
@chr-hertel
Copy link
Contributor

maybe we should add a test to ensure channel is optional? not sure if it fits better in factory or transport test 🤔

Copy link
Contributor

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

LGTM

@fabpot
Copy link
Member

fabpot commented Nov 30, 2019

Thank you @Jeroeny.

fabpot added a commit that referenced this pull request Nov 30, 2019
This PR was squashed before being merged into the 5.0 branch (closes #34539).

Discussion
----------

[Notifier] Add telegram tests

| Q             | A
| ------------- | ---
| Branch?       | 5.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | See #33687
| License       | MIT

Adds tests for the Telegram bridge of the Notifier component. Not sure if this is the right way to go, but would love some feedback.

#SymfonyHackday

Commits
-------

3b4d83c [Notifier] Add telegram tests
@fabpot fabpot merged commit 3b4d83c into symfony:5.0 Nov 30, 2019
@Jeroeny Jeroeny deleted the telegram-tests branch December 2, 2019 08:15
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants