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

Simplifications in remote_control #778

Merged
merged 7 commits into from
Jul 10, 2024
Merged

Conversation

zachallaun
Copy link
Collaborator

Commits are atomic and can be reviewed individually.

@zachallaun zachallaun requested review from scohen and scottming June 19, 2024 14:36
@zachallaun zachallaun force-pushed the za-simplify-remote-control-app branch from 0d379a4 to 99d9b0f Compare June 25, 2024 13:48
@scohen
Copy link
Collaborator

scohen commented Jul 10, 2024

Left one comment, otherwise it looks good.

@zachallaun zachallaun force-pushed the za-simplify-remote-control-app branch from 99d9b0f to 466119b Compare July 10, 2024 14:10
`ModuleMappings.update/2` was only used in the tests. I've refactored them to use dispatched messages, which is how the mappings are actually updated during regular use.
It was only used for two calls and was difficult to understand. Instead, we now explicitly pass an mfa tuple to `RemoteControl.Modules.with_prefix/2` and the module being testing is added as the first arg.
@zachallaun zachallaun force-pushed the za-simplify-remote-control-app branch from 466119b to 60c8701 Compare July 10, 2024 14:15
@zachallaun zachallaun merged commit a5be8d1 into main Jul 10, 2024
12 checks passed
@zachallaun zachallaun deleted the za-simplify-remote-control-app branch July 10, 2024 14:18
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.

2 participants