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

DNRY: add .flush() method #12

Closed
wants to merge 5 commits into from
Closed

DNRY: add .flush() method #12

wants to merge 5 commits into from

Conversation

kklash
Copy link
Contributor

@kklash kklash commented Jul 2, 2020

Simplifies the duplicated pattern of writing the seco-keyval instance to disk in one tidy async method.

@kklash kklash requested a review from RyanZim July 2, 2020 00:05
@kklash kklash self-assigned this Jul 2, 2020
README.md Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
kklash and others added 2 commits July 2, 2020 13:35
Co-authored-by: Mark Vayngrib <mvayngrib@gmail.com>
Co-authored-by: Mark Vayngrib <mvayngrib@gmail.com>
@kklash
Copy link
Contributor Author

kklash commented Jul 2, 2020

Thanks @mvayngrib !

Copy link
Contributor

@mvayngrib mvayngrib left a comment

Choose a reason for hiding this comment

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

utACK

@RyanZim
Copy link
Collaborator

RyanZim commented Jul 4, 2020

@kklash Still needed?

@kklash
Copy link
Contributor Author

kklash commented Jul 6, 2020

@RyanZim Probably not 100% necessary but it seems like a good refactor for simplification if you'd like me to do it

@RyanZim
Copy link
Collaborator

RyanZim commented Jul 6, 2020

Doesn't matter to me. Debatable if it should be a public method, though.

@kklash kklash marked this pull request as draft July 6, 2020 21:22
@RyanZim
Copy link
Collaborator

RyanZim commented Sep 15, 2020

@kklash should we close this?

@kklash kklash closed this Sep 15, 2020
@kklash kklash deleted the konnor/flush branch September 15, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants