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

Return subscription for on method #5080

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Return subscription for on method #5080

merged 4 commits into from
Nov 20, 2024

Conversation

HarelM
Copy link
Collaborator

@HarelM HarelM commented Nov 20, 2024 •
edited
Loading

This changes the return value of on method from being chainable to something that returns a subscription that allows to unsubscribe easily.

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Add an entry to CHANGELOG.md under the ## main section.

@HarelM HarelM mentioned this pull request Nov 20, 2024
Open
16 tasks
birkskyum
birkskyum approved these changes Nov 20, 2024
View reviewed changes
Copy link
Member

@birkskyum birkskyum left a comment

Choose a reason for hiding this comment

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

Great idea

@HarelM
Copy link
Collaborator Author

HarelM commented Nov 20, 2024

I also replaced a lot of the "done" in the tests, still like 150 to go, but I'm slowly removing them...

@HarelM HarelM enabled auto-merge (squash) November 20, 2024 15:39
@HarelM HarelM merged commit c46a804 into main Nov 20, 2024
16 of 17 checks passed
@HarelM HarelM deleted the return-subscription branch November 20, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@birkskyum birkskyum birkskyum approved these changes

@ibesora ibesora Awaiting requested review from ibesora

@sbachinin sbachinin Awaiting requested review from sbachinin

@neodescis neodescis Awaiting requested review from neodescis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@HarelM @birkskyum