Skip to content
This repository was archived by the owner on Jan 21, 2022. It is now read-only.

Write only db - #234

Open
lruggieri wants to merge 1 commit into
boramalper:masterfrom
lruggieri:write_only_db
Open

Write only db#234
lruggieri wants to merge 1 commit into
boramalper:masterfrom
lruggieri:write_only_db

Conversation

@lruggieri

Copy link
Copy Markdown
Contributor

Adding interface for write-only type DBs. This adds to @skobkin Beanstalk engine implementation.

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

Comment thread pkg/persistence/interface.go Outdated
Comment thread pkg/persistence/interface.go Outdated
Comment thread pkg/persistence/interface.go
Comment thread pkg/persistence/interface.go Outdated
@skobkin

skobkin commented Nov 14, 2019

Copy link
Copy Markdown
Contributor

@lruggieri When I said that it must be proposed to the upstream I also mentioned change segregation. I meant that my changes adding beanstalkd support must not be included in such PR.

So I think it'd be better to rebase this PR to only implement this interface for stdout engine and exclude all of my beanstalkd changes. So basically I mean that this PR must change only interface.go and stdout.go.

This way if @boramalper want to merge your changes and don't want to merge mine it is possible for him to do that.

Someone correct me if I'm wrong.

BTW I like the idea of second interface for write-only engines so we don't need to implement methods which we don't need in such implementations.

@skobkin

skobkin commented Nov 14, 2019

Copy link
Copy Markdown
Contributor

Yes. Now that's more like what I've suggested earlier.

I'm not sure about the code though. Leaving this to @boramalper who knows Go far better than me.

@lruggieri

Copy link
Copy Markdown
Contributor Author

@skobkin makes sense, even if it is will be a bit long and I am starting to have a bit too many branches on my fork xD
Anyway, this should do it. Check it out when you have time

@skobkin

skobkin commented Nov 14, 2019

Copy link
Copy Markdown
Contributor

I am starting to have a bit too many branches on my fork xD

You can do what I did. I've just merged all I need for my project to the separate branch and building Docker images from there. But each feature is also in the separate branch which makes it simpler to review or merge for upstream maintainer.

Check it out when you have time

As I said earlier I have some doubts about the code itself. But I'm not Go expert and do not own this project. So it'd be better for Bora to do code review.

Regarding change/feature separation I think everything is fine now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants