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.

Sign up for GitHub

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

Jump to bottom

chore: extract observer loop and add tests #81

Merged
merged 11 commits into from
Feb 6, 2025
Merged

chore: extract observer loop and add tests #81

merged 11 commits into from
Feb 6, 2025

Conversation

NikolasHaimerl
Copy link
Contributor

@NikolasHaimerl NikolasHaimerl commented Feb 4, 2025 •
edited
Loading

This PR proposes the following changes:

  1. Extract the functionality of deal observer loop into its own function to be called by the loop
  2. Test the extracted functionality

Closes #61 .
See implementation plan.

@NikolasHaimerl NikolasHaimerl mentioned this pull request Feb 4, 2025
Open
14 tasks
Nikolas Haimerl added 2 commits February 4, 2025 15:04
@NikolasHaimerl NikolasHaimerl marked this pull request as ready for review February 4, 2025 15:08
@juliangruber

This comment was marked as resolved.

Sign in to view
juliangruber
juliangruber requested changes Feb 5, 2025
View reviewed changes
backend/lib/deal-observer.js Outdated Show resolved Hide resolved
pyropy
pyropy reviewed Feb 5, 2025
View reviewed changes
backend/lib/deal-observer.js Outdated Show resolved Hide resolved
@juliangruber
Copy link
Member

@NikolasHaimerl FYI:

Screenshot 2025-02-06 at 10 24 13 Screenshot 2025-02-06 at 10 24 20

Here I picked the last option, "Resolved". This helps keep the issue clean

@juliangruber juliangruber changed the title chore: extract obxerver loop and add tests chore: extract observer loop and add tests Feb 6, 2025
juliangruber
juliangruber requested changes Feb 6, 2025
View reviewed changes
backend/lib/deal-observer.js Show resolved Hide resolved
backend/test/deal-observer.test.js Outdated Show resolved Hide resolved
backend/test/deal-observer.test.js Outdated Show resolved Hide resolved
backend/test/deal-observer.test.js Outdated Show resolved Hide resolved
juliangruber
juliangruber approved these changes Feb 6, 2025
View reviewed changes
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

👏

@NikolasHaimerl NikolasHaimerl merged commit 85b7cb3 into main Feb 6, 2025
8 checks passed
@NikolasHaimerl NikolasHaimerl deleted the nhaimerl-test-deal-observer-main-loop branch February 6, 2025 13:58
@sentry-io Sentry.io
Copy link

sentry-io bot commented Feb 6, 2025 •
edited
Loading

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: Failed to make RPC request Filecoin.ChainHead rpcRequest(service) View Issue
  • ‼️ Error: Failed to make RPC request Filecoin.ChainHead rpcRequest(service) View Issue
  • ‼️ Error: Connection terminated unexpectedly loadDeals(deal-observer) View Issue
  • ‼️ Error: Error inserting deals storeActiveDeals(deal-observer) View Issue
  • ‼️ Error: Error inserting deals storeActiveDeals(deal-observer) View Issue

Did you find this useful? React with a 👍 or 👎

@NikolasHaimerl NikolasHaimerl mentioned this pull request Feb 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pyropy pyropy pyropy left review comments

@juliangruber juliangruber juliangruber approved these changes

@bajtos bajtos Awaiting requested review from bajtos bajtos is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test Main Loop Function
3 participants
@NikolasHaimerl @juliangruber @pyropy