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

[IND-288]: Remove unused socks fields and add candle resolutions for websockets #633

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

Christopher-Li
Copy link
Contributor

Changelist

  • Remove fields in socks that weren't being sent to users, and sent candle resolutions through websockets, because previously it was expected that candle resolution was sent

Test Plan

unit tests

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2023

Walkthrough

The changes primarily focus on simplifying the codebase by consolidating message types, removing unused imports, and adjusting object properties in various functions. The modifications also include the addition of a resolution property to the CandleMessageContents interface and the removal of the CandleColumns.id property from certain test functions.

Changes

File Path Summary
.../postgres/src/types/websocket-message-types.ts Added CandleResolution import and resolution property to CandleMessageContents interface.
.../ender/__tests__/lib/candles-generator.test.ts Removed CandleColumns.id property from expectedCandle object in verifyAllCandlesEqualsKafkaMessages and verifyContainsCandlesKafkaMessages functions.
.../ender/src/lib/candles-generator.ts Removed CandleColumns.resolution property from _.omit function call in candleMessageContents object.
.../socks/__tests__/helpers/from-kafka-helpers.test.ts Removed unused imports and type declarations, and specific type assertions when calling getMessageToForward() function.
.../socks/src/helpers/from-kafka-helpers.ts Removed certain properties from objects in getMessageToForward function.
.../socks/src/types.ts Consolidated several specific message types into a single MessageToForward type and removed MessageToForwardBase interface and CandleResolution import.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between a5a391d and b7f2f32.
Files selected for processing (6)
  • indexer/packages/postgres/src/types/websocket-message-types.ts (2 hunks)
  • indexer/services/ender/tests/lib/candles-generator.test.ts (2 hunks)
  • indexer/services/ender/src/lib/candles-generator.ts (1 hunks)
  • indexer/services/socks/tests/helpers/from-kafka-helpers.test.ts (5 hunks)
  • indexer/services/socks/src/helpers/from-kafka-helpers.ts (4 hunks)
  • indexer/services/socks/src/types.ts (2 hunks)
Files skipped from review due to trivial changes (4)
  • indexer/services/ender/src/lib/candles-generator.ts
  • indexer/services/socks/tests/helpers/from-kafka-helpers.test.ts
  • indexer/services/socks/src/helpers/from-kafka-helpers.ts
  • indexer/services/socks/src/types.ts
Additional comments (Suppressed): 4
indexer/services/ender/__tests__/lib/candles-generator.test.ts (2)
  • 480-485: The CandleColumns.resolution property is no longer omitted from the candleMessageContents object. This change is consistent with the PR summary. However, ensure that this change does not affect other parts of the code that rely on the candleMessageContents object.

  • 516-521: The CandleColumns.resolution property is no longer omitted from the expectedCandle object. This change is consistent with the PR summary. However, ensure that this change does not affect other parts of the code that rely on the expectedCandle object.

indexer/packages/postgres/src/types/websocket-message-types.ts (2)
  • 1-4: The import of CandleResolution from ./candle-types is new. Ensure that the CandleResolution type is correctly defined in the candle-types file and is compatible with its usage in the CandleMessageContents interface.

  • 227-234: The CandleMessageContents interface now includes a resolution property of type CandleResolution. This is a change from the previous version where the resolution was not included in the CandleMessageContents interface. Make sure that all instances where CandleMessageContents is used have been updated to include the resolution property.

@Christopher-Li Christopher-Li merged commit 4906fc4 into main Oct 16, 2023
@Christopher-Li Christopher-Li deleted the cl_clean_up_socks branch October 16, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants