Localize signal snapshot source in notifications#51
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df7b510eb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| localize_price_source_label as _localize_price_source_label, | ||
| localize_quote_overlay_state as _localize_quote_overlay_state, |
There was a problem hiding this comment.
Keep imports compatible with pinned QuantPlatformKit
In the CI and Docker paths I checked, the app installs requirements.txt, which pins QuantPlatformKit to 86d72631c76f27c668b234704b91241bec8fa9ee; that version's common.notification_localization only exports localize_notification_text, not these two new helpers. With the pinned dependency, importing notifications.telegram now raises ImportError before any notification code can run, so either the requirements pin needs to be advanced with this change or these helpers need a local compatibility fallback.
Useful? React with 👍 / 👎.
df7b510 to
bfacaeb
Compare
Summary
Verification