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

new sliding window by time function #374

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

new sliding window by time function #374

wants to merge 8 commits into from

Conversation

sorumehta
Copy link

@sorumehta sorumehta commented May 12, 2024 •
edited
Loading

Implemented sliding window functionality

@sorumehta sorumehta requested a review from a team as a code owner May 12, 2024 07:10
@sorumehta sorumehta mentioned this pull request May 12, 2024
Open
@kidpollo
Copy link
Contributor

Dont forget to add tests :D https://github.com/FundingCircle/jackdaw/blob/master/test/jackdaw/streams_test.clj

@sorumehta
Copy link
Author

@kidpollo I have (finally) added the test. Also looked at my implementation and made some modifications. Let me know what you think

@sorumehta sorumehta requested a review from kidpollo July 9, 2024 00:34
@sorumehta
Copy link
Author

It doesn't pass the tests. We are getting intermediate values from the windows. Will try suppressing those until the windows close

@sorumehta
Copy link
Author

@kidpollo This is done, tests are now passing.

kidpollo
kidpollo approved these changes Jul 9, 2024
View reviewed changes
Copy link
Contributor

@kidpollo kidpollo left a comment

Choose a reason for hiding this comment

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

LGTM

@sorumehta
Copy link
Author

LGTM

@kidpollo Great, thanks! Hope it will get merged too!

@sorumehta
Copy link
Author

Any updates here? @kidpollo

@kidpollo
Copy link
Contributor

Unfortunately I am no longer an FC employee and have no insight on who actually runs this project :P maybe @jbropho ?

@sorumehta
Copy link
Author

@jbropho Can you review the PR?

@sorumehta
Copy link
Author

@kidpollo Anyone else you can think of for reviewing the PR?

@kidpollo
Copy link
Contributor

kidpollo commented Aug 12, 2024 •
edited
Loading

Unfortunately no. If you are super eager you could go down the list of contributor, however most dont work for FC. I feel that if this project has enough other users it should be given back to the community. Easy said ...

@sorumehta
Copy link
Author

@rads or @aprobus Can you review this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kidpollo kidpollo kidpollo approved these changes

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
2 participants
@sorumehta @kidpollo