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

refactor(queue_store): rename queue_store module and simplify api #1348

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Nov 7, 2022

This PR is part of the REST API, c bindings, and Waku Store roadmaps. These changes are accessories and help to settle the bases to decouple the RPC types from the API types for the Waku Store protocol.

  • Simplify the StoreQueueRef API
  • Decouple the StoreQueueRef from the Waku Store RPC errors
  • Fix MessageStore's "no methods without base" warning
  • Rework and update queue store's test cases accordingly

@LNSD LNSD requested review from alrevuelta, rymnc and jm-clius November 7, 2022 15:34
@LNSD LNSD self-assigned this Nov 7, 2022
@@ -168,130 +168,6 @@ procSuite "Sorted store queue":
lastRes.isErr()
lastRes.error() == "Not found"

test "Store queue pagination works with predicate - fwd direction":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of these test cases were overlapping with the ones in test_message_store_queue_pagination. Removed the duplicated ones, and added the rest of the pagination-related test cases to the other test suite.

@status-im-auto
Copy link
Collaborator

status-im-auto commented Nov 7, 2022

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ c90ef6e #1 2022-11-07 15:49:30 ~14 min macos 📦bin
c90ef6e #1 2022-11-07 15:51:58 ~16 min linux 📄log
✔️ c90ef6e #2 2022-11-07 16:15:41 ~17 min linux 📦bin

Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@LNSD
Copy link
Contributor Author

LNSD commented Nov 8, 2022

I have re-run the failed check (macOS test2 job timed out again). If it does not pass this time, I will merge this PR.

The failure cause is unrelated to this PR's changes.

@LNSD LNSD merged commit c4f9813 into master Nov 8, 2022
@LNSD LNSD deleted the chore-queuestore-rename branch November 8, 2022 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants