-
Notifications
You must be signed in to change notification settings - Fork 16
fix: adjust initialize logic #370
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
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
WalkthroughCalls to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/apps/base.py (1)
266-271: Combine if branches for cleaner code.Both branches call the same function. Combine them.
- if passphrase_state and se_thd89.check_passphrase_btc_test_address( - passphrase_state - ): - session_id = storage.cache.start_session() - elif has_attach and session_id_in_msg is not None and passphrase_state is None: + if (passphrase_state and se_thd89.check_passphrase_btc_test_address(passphrase_state)) or (has_attach and session_id_in_msg is not None and passphrase_state is None): session_id = storage.cache.start_session()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (5)
core/src/apps/base.py(1 hunks)core/src/apps/homescreen/lockscreen.py(0 hunks)core/src/apps/webauthn/fido2.py(0 hunks)core/src/boot.py(0 hunks)core/src/trezor/uart.py(0 hunks)
💤 Files with no reviewable changes (4)
- core/src/apps/homescreen/lockscreen.py
- core/src/boot.py
- core/src/trezor/uart.py
- core/src/apps/webauthn/fido2.py
🧰 Additional context used
🪛 Ruff (0.12.2)
core/src/apps/base.py
266-271: Combine if branches using logical or operator
Combine if branches
(SIM114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Gen check
- GitHub Check: Style check
- GitHub Check: Defs check
🔇 Additional comments (2)
core/src/apps/base.py (2)
250-252: Variable extraction improves readability.Clean refactor that makes the code easier to follow.
257-263: Type handling is thorough and defensive.Good improvement for handling different passphrase_state types.
80950ec to
9618f46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/apps/base.py (1)
266-271: Consider combining if branches.Both conditions call
start_session()without arguments.- if passphrase_state and se_thd89.check_passphrase_btc_test_address( - passphrase_state - ): - session_id = storage.cache.start_session() - elif has_attach and session_id_in_msg is not None and passphrase_state is None: - session_id = storage.cache.start_session() + if (passphrase_state and se_thd89.check_passphrase_btc_test_address(passphrase_state)) or (has_attach and session_id_in_msg is not None and passphrase_state is None): + session_id = storage.cache.start_session()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
core/src/apps/base.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
core/src/apps/base.py
266-271: Combine if branches using logical or operator
Combine if branches
(SIM114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Gen check
- GitHub Check: Defs check
- GitHub Check: Style check
🔇 Additional comments (2)
core/src/apps/base.py (2)
250-252: Good refactor for readability.Extracting the condition to
has_attachimproves code clarity.
257-263: Robust type handling.The passphrase state extraction properly handles bytes, strings, and other types.
9618f46 to
c8b8f50
Compare
f933060 to
2e35f76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/apps/base.py (1)
265-274: Combine if branches as suggested by static analysis.The logic can be simplified by combining the conditions.
- session_id_in_msg = getattr(msg, "session_id", None) - if passphrase_state and se_thd89.check_passphrase_btc_test_address( - passphrase_state - ): - session_id = storage.cache.start_session() - elif has_attach and session_id_in_msg is not None and passphrase_state is None: - session_id = storage.cache.start_session() - else: - session_id = storage.cache.start_session(session_id_in_msg) + session_id_in_msg = getattr(msg, "session_id", None) + if (passphrase_state and se_thd89.check_passphrase_btc_test_address(passphrase_state)) or (has_attach and session_id_in_msg is not None and passphrase_state is None): + session_id = storage.cache.start_session() + else: + session_id = storage.cache.start_session(session_id_in_msg)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
core/src/apps/base.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
core/src/apps/base.py
266-271: Combine if branches using logical or operator
Combine if branches
(SIM114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Gen check
- GitHub Check: Defs check
- GitHub Check: Style check
🔇 Additional comments (1)
core/src/apps/base.py (1)
250-264: Improve passphrase state extraction robustness.The new logic handles both bytes and string types for
passphrase_state, which is good defensive programming.
Summary by CodeRabbit