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

Extract reducers to common package #17090

Open
wants to merge 12 commits into
base: feat/rust-bluetooth
Choose a base branch
from

Conversation

peter-sanderson
Copy link
Contributor

@peter-sanderson peter-sanderson commented Feb 18, 2025

No description provided.

Copy link

github-actions bot commented Feb 18, 2025

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 26
  • More info

Learn more about 𝝠 Expo Github Action

@peter-sanderson peter-sanderson added the no-project This label is used to specify that PR doesn't need to be added to a project label Feb 18, 2025
@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch 2 times, most recently from 06d724b to 8bf10a7 Compare February 19, 2025 10:52
@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch from 3049fae to fe5aa32 Compare February 19, 2025 16:43
@peter-sanderson peter-sanderson marked this pull request as ready for review February 19, 2025 16:44
@@ -35,17 +33,21 @@ type BluetoothConnectProps = {
uiMode: 'spatial' | 'card';
};

const selectAllDevices = prepareSelectAllDevices<BluetoothDevice>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has to be done to create generic selector with memoized output. Consulted with @Nodonisko

@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch from fe5aa32 to c0009aa Compare February 19, 2025 16:49
@Lemonexe Lemonexe self-requested a review February 20, 2025 11:21
@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch from daf8447 to 6d9cf75 Compare February 20, 2025 12:15
@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch from 6d9cf75 to e1e8026 Compare February 20, 2025 13:16
Copy link
Contributor

@Lemonexe Lemonexe left a comment

Choose a reason for hiding this comment

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

CR still in progress; dropping off comments, mostly nits.

@peter-sanderson peter-sanderson force-pushed the bluetooth-common-reducer2 branch from 72fdc6b to 20fcc71 Compare February 21, 2025 12:05
Copy link
Contributor

@Lemonexe Lemonexe left a comment

Choose a reason for hiding this comment

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

LGTM. Big amount of work 💪 🚀
I've got some nits and questions (see comments), but I'm not yet very familiar with the topic, so they may be irrelevant.
➡️ feel free to resolve them in a way that you deem suitable.

Also I did not test, because I don't have a BT device, and also because it's not merged into develop. I expect that big QA effort shall be done on feat/rust-bluetooth before merging that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-project This label is used to specify that PR doesn't need to be added to a project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants