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

Rename dispatch to send for consistency with TCA #27

Closed
wants to merge 1 commit into from

Conversation

jonekdahl
Copy link

Summary

As discussed in #23, using send instead of dispatch would be beneficial for individuals and teams that have previous experience with TCA (The Composable Architecture). This PR changes the name to send, deprecating the dispatch methods.

Relationships

Closes #23

Technical considerations

Tests

Existing tests have been updated. No new tests have been added since this is just a rename that doesn't introduce any new behavior.

Review pointers

As suggested by @semanticer, @Deprecated annotations were added, I hope I did them correctly.

@semanticer semanticer self-requested a review March 15, 2022 15:10
Copy link
Collaborator

@semanticer semanticer left a comment

Choose a reason for hiding this comment

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

I'm sorry for the late review and thanks again for this valuable contribution.

Nothing seems to be broken, all the changes are correct IMO and I tested whether @Deprecation annotation works and it does 👍

However, by searching the whole project for "dispatch" I discovered many other places we should probably fix. It's mostly in the docs, comments or tests but we should also probably rename those
dispatchFn: (List<Action>) -> Unit
as well right? @toggl/androids

@jonekdahl
Copy link
Author

Hi @semanticer! Are you expecting feedback from @toggl/androids or can I go ahead and do the changes you propose?

@semanticer
Copy link
Collaborator

Hi @semanticer! Are you expecting feedback from @toggl/androids or can I go ahead and do the changes you propose?

Go for it, and thanks again!

@pdrj
Copy link
Collaborator

pdrj commented Mar 28, 2022

Sorry that it's taking so long @jonekdahl

It's mostly in the docs, comments or tests but we should also probably rename those
dispatchFn: (List) -> Unit
as well right?

Yeah, it'd be good to have these changed as well to avoid confusion.

@semanticer
Copy link
Collaborator

Hey @jonekdahl should I finish this or do you want to do the last changes yourself?

@jonekdahl
Copy link
Author

jonekdahl commented May 5, 2022

@semanticer Go ahead!

@semanticer
Copy link
Collaborator

Here is the continuation #30

@semanticer semanticer closed this May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Naming convention
3 participants