Skip to content
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

[Notifier] Create DesktopChannel and JoliNotif bridge #20076

Merged
merged 1 commit into from
Aug 28, 2024
Merged

[Notifier] Create DesktopChannel and JoliNotif bridge #20076

merged 1 commit into from
Aug 28, 2024

Conversation

ahmedghanem00
Copy link
Contributor

@ahmedghanem00 ahmedghanem00 commented Jul 29, 2024 •
edited by OskarStark
Loading

Updating docs according to symfony/symfony#57683

Fixes #20168

@carsonbot carsonbot added this to the 7.2 milestone Jul 29, 2024
@OskarStark OskarStark added the Waiting Code Merge Docs for features pending to be merged label Jul 29, 2024
@carsonbot carsonbot modified the milestones: 7.2, next Jul 29, 2024
javiereguiluz
javiereguiluz approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

This looks correct to me ... but I'm not an expert here, so I'll wait for more approvals before merging. Thanks!

@javiereguiluz javiereguiluz mentioned this pull request Aug 27, 2024
Closed
OskarStark
OskarStark approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Looks good to me, but a versioned directive is missing for 7.2

@ahmedghanem00
Copy link
Contributor Author

Looks good to me, but a versioned directive is missing for 7.2

Can you provide an example for this directive? :)

@OskarStark
Copy link
Contributor

Can you provide an example for this directive? :)

Done sorry :-)

@OskarStark OskarStark removed the Waiting Code Merge Docs for features pending to be merged label Aug 28, 2024
OskarStark
OskarStark reviewed Aug 28, 2024
View reviewed changes
notifier.rst Outdated Show resolved Hide resolved
OskarStark
OskarStark approved these changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Good to merge after my two comments

@OskarStark OskarStark modified the milestones: next, 7.2 Aug 28, 2024
@javiereguiluz javiereguiluz merged commit 4203d31 into symfony:7.2 Aug 28, 2024
1 of 3 checks passed
@javiereguiluz
Copy link
Member

Merged! Ahmed, thanks a lot for this contribution.

@ahmedghanem00 ahmedghanem00 deleted the patch-1 branch August 28, 2024 11:43
@javiereguiluz javiereguiluz mentioned this pull request Aug 28, 2024
Merged
javiereguiluz added a commit that referenced this pull request Aug 28, 2024
@javiereguiluz
minor #20170 [Notifier] Misc. minor tweaks (javiereguiluz)
4d9812c 
This PR was merged into the 6.4 branch.

Discussion
----------

[Notifier] Misc. minor tweaks

After merging #20076 I realized that we don't really explain what `channels` are in the context of the Notifier. Also, I propose to reorder some items to improve reading flow.

Commits
-------

398f6d1 [Notifier] Misc. minor tweaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark approved these changes

@javiereguiluz javiereguiluz javiereguiluz approved these changes

Assignees
No one assigned
Labels
Notifier Status: Reviewed
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
4 participants
@ahmedghanem00 @OskarStark @javiereguiluz @carsonbot