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

moved in bitcask schema bits from riak.schema #112

Merged
merged 2 commits into from
Oct 15, 2013
Merged

Conversation

joedevivo
Copy link
Contributor

needs cuttlefish from branch jd-test-tools

see: basho/cuttlefish#44

@slfritchie
Copy link
Contributor

Just as a general note ... there are several apps using Bitcask besides Riak.

@evanmcc
Copy link
Contributor

evanmcc commented Oct 10, 2013

I asked Joe about this in the core/kv channel earlier today, he said that
other than the additional dep for rebar-using projects, nothing should
change for non-riak projects.

@jrwest
Copy link

jrwest commented Oct 10, 2013

I reviewed the riak_core changes this morning now but I'm wondering:

Wasn't one of the initial reasons for keeping this in riak to be as non-intrusive as possible -- although this is really very little intrusion just being an extra dep? But would it be possible, and do we achieve both goals, by just breaking up the giant schema inside of basho/riak into multiple files and having those and all the tests in a riak_config (or otherwise named) dependency pulled into both OSS and Enterprise?*

*this suggestion may be prone to tired crazy talk

@joedevivo
Copy link
Contributor Author

@jrwest you crazy!

But seriously folks, I am confident that this is the way to go. I only wish that rebar had a "test" scope for dependencies.

Other than running the unit tests for the schema, this will have no effect on non-cuttlefish compatible projects.

There will be stuff that remains in the riak and riak-ee repositories, but the less that does, the less duplication we'll have to deal with.

@joedevivo
Copy link
Contributor Author

For the record, @jrwest and I had a mumble and he's cool with this now :)

]}.


%% @doc Require the CRC to be present at the end of hintfiles.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should be updated, since you've (rightly) changed the default to true. That also reminds me I need to fix this so it actually works for 2.0 (see issue #101).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushed new wording, let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

s/them/CRC signatures/ and merge it

@evanmcc
Copy link
Contributor

evanmcc commented Oct 15, 2013

Other than the one comment, unit tests pass and values have been transcribed accurately, it looks like. Once the comment is addressed, I think that this can be merged, although like all of the schema changes, it deserves some more strenuous testing when fully integrated.

👍

joedevivo added a commit that referenced this pull request Oct 15, 2013
moved in bitcask schema bits from riak.schema
@joedevivo joedevivo merged commit 3fac8e9 into develop Oct 15, 2013
@seancribbs seancribbs deleted the jd-cuttlefish branch April 1, 2015 22:34
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.

None yet

4 participants