-
Notifications
You must be signed in to change notification settings - Fork 3
chore: handle pricing when endtime is in future #112
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
📝 WalkthroughWalkthroughThe changes modify the ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
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)
packages/pricing/src/providers/coingecko.provider.ts (1)
119-124
: LGTM! Properly handles future timestamps.The implementation correctly adjusts the time range when
endTimestampMs
is in the future, ensuring that Coingecko API calls only use past timestamps. This prevents potential errors or incomplete data from the API.Consider extracting the buffer value (60 * 1000) to a named constant for better readability:
// Handle when the endTimestampMs is in the future -const currentTimestamp = Date.now() - 60 * 1000; +const TIMESTAMP_BUFFER_MS = 60 * 1000; // One minute buffer +const currentTimestamp = Date.now() - TIMESTAMP_BUFFER_MS; if (currentTimestamp < endTimestampMs) { startTimestampMs = (currentTimestamp - TIME_DELTA) as TimestampMs; endTimestampMs = currentTimestamp as TimestampMs; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/pricing/src/providers/coingecko.provider.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`:
**/*.ts
:
packages/pricing/src/providers/coingecko.provider.ts
`**/providers/**/*.ts`: Review provider files for the following: - Providers should supply narrowly scoped data/resources. - Ensure classes interacting with metadata sources (e...
**/providers/**/*.ts
: Review provider files for the following:
- Providers should supply narrowly scoped data/resources.
- Ensure classes interacting with metadata sources (e.g., GitHub, JSON
files, IPFS) implement theIMetadataProvider
interface and follow
naming conventions (e.g.,GithubProvider
,JsonFileProvider
).- Be concise and avoid overly nitpicky feedback outside of these best practices.
packages/pricing/src/providers/coingecko.provider.ts
🧬 Code Definitions (1)
packages/pricing/src/providers/coingecko.provider.ts (1)
packages/shared/src/types/common.ts (1)
TimestampMs
(9-9)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: e2e-tests / E2E Tests
🤖 Linear
Closes PAR-XXX
Description
Checklist before requesting a review
Summary by CodeRabbit