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.

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

feat: allow null when subscribing notification #33641

Merged
merged 2 commits into from Apr 13, 2022
Merged

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Apr 6, 2022

Description of Change

Closes #33632.

Allows for systemPreferences.subscribe{Local|Workspace}Notification methods to take null for their event parameters.

Per documentation:

When nil, the notification center doesn’t use a notification’s name to decide whether to deliver it to the observer.

cc @knev

Checklist

Release Notes

Notes: Enabled systemPreferences.subscribe{Local|Workspace}Notification to take a null value for the event parameter.

@codebytere codebytere added semver/minor backwards-compatible functionality target/17-x-y labels Apr 6, 2022
@electron-cation electron-cation bot added api-review/requested 🗳 new-pr 🌱 PR opened in the last 24 hours labels Apr 6, 2022
@codebytere codebytere changed the title feat: allow null when subscribing notification feat: allow null when subscribing notification Apr 6, 2022
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

API LGTM, but have code / docs comments.

docs/api/system-preferences.md Show resolved Hide resolved
@arslan-raza-143

This comment was marked as spam.

@codebytere codebytere requested a review from nornagon April 7, 2022 17:50
@miniak
Copy link
Contributor

miniak commented Apr 8, 2022

API LGTM

1 similar comment
@nornagon
Copy link
Member

API LGTM

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

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

API LGTM

@codebytere codebytere merged commit b66667b into main Apr 13, 2022
@codebytere codebytere deleted the allow-null-subscribe branch April 13, 2022 20:02
@release-clerk
Copy link

release-clerk bot commented Apr 13, 2022

Release Notes Persisted

Enabled systemPreferences.subscribe{Local|Workspace}Notification to take a null value for the event parameter.

@trop
Copy link
Contributor

trop bot commented Apr 13, 2022

I was unable to backport this PR to "17-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Apr 13, 2022

I have automatically backported this PR to "19-x-y", please check out #33770

@trop
Copy link
Contributor

trop bot commented Apr 13, 2022

I have automatically backported this PR to "18-x-y", please check out #33771

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 18, 2022
bavulapati pushed a commit to bavulapati/electron that referenced this pull request Apr 29, 2022
* feat: allow null when subscribing notification

* docs: document null event
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* feat: allow null when subscribing notification

* docs: document null event
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Name = nil is a valid input for NSDistributedNotificationCenter on macOS
5 participants