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

Add dvv_enabled to .proto definition and codec #127

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

kuenishi
Copy link
Contributor

This pull request is for making basho/riak_cs#1176 finally work. Not mandatory for Riak CS 2.1 release, but hope this merged in 2.1. Do I have to open this against 2.1 branch?

@kuenishi
Copy link
Contributor Author

mmm, I don't understand why build is failing with this:

riak_pb_codec.erl:350: Invalid type specification for function riak_pb_codec:decode_commit_hooks/1. The success typing is ([any()]) -> [{atom(),atom() | [any(),...]} | {'modfun',atom(),atom() | [any(),...]}]
make: *** [dialyzer-run] Error 1

@kuenishi
Copy link
Contributor Author

This is now working. Ready for review again :)

@mitchellwrosen
Copy link

@kuenishi Still interested in making this happen?

@kuenishi
Copy link
Contributor Author

@mitchellwrosen I no longer work for Basho and have no permission in this organization. Maybe @russelldb or @martinsumner be interested?

@martinsumner
Copy link
Contributor

martinsumner commented Aug 29, 2018

@mitchellwrosen you may may wish to appeal for help on postriak.slack.com where Nick Adams and the crew from tiote are active (and have been working on new CS releases). Slack seems to be down for me at the moment, but I will try and find someone to help later.

[EDIT]
Ah, I'm on now and I see you're on there - nadams (tiote) and mcx (bet365) may be best placed to help

@Bob-The-Marauder
Copy link

I was reviewing the Riak CS pull requests and noticed this one required the protocol buffers to be patched first. Any thoughts on this?

@martinsumner
Copy link
Contributor

Given that ultimately all it does is warn if dvv_enabled is false, I'm not willing to restart the test cycle to add this in to the 3.0.2 release at this stage. The by-default enabling of dvv has been around for a long time.

Perhaps this might be better resolved by actually enforcing dvv_enabled to be true, and removing the optionality going forward in 3.1 and beyond. There are perhaps a number of other optional configurations (e.g. object format, aae versions) for legacy backwards compatibility that we might want to sweep up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants