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

compile in 17.0: clients are compiles with various OTP versions #89

Merged
merged 2 commits into from
Apr 30, 2014

Conversation

kuenishi
Copy link
Contributor

No description provided.

@kuenishi kuenishi mentioned this pull request Apr 24, 2014
4 tasks
@@ -49,7 +49,7 @@
-type quorum() :: symbolic_quorum() | non_neg_integer().
-type symbolic_quorum() :: one | quorum | all | default.
-type value() :: binary().
-type metadata() :: dict().
-type metadata() :: dict:dict(binary(), binary()).

Choose a reason for hiding this comment

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

Isn't this type invalid on releases < 17.0? Furthermore, dict:dict/0 is the type in previous releases, not dict:dict/2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's invalid. I'll fix it in the following commit by introducing a sad version-specific switch. And to give nowarn_... to dialyzer we have to modify tools.mk, which probably we hope to be same across all our repositories?

Choose a reason for hiding this comment

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

No, you can specify the flags to use in the DIALYZER_FLAGS variable: https://github.com/basho/tools.mk/blob/master/tools.mk#L17

@seancribbs
Copy link

I'd rather see this include a version-specific switch so that our specs can be valid on both releases.

@seancribbs
Copy link

Example, on R16B02-basho4:

Unknown types:
  dict:dict/2

@evanmcc
Copy link
Contributor

evanmcc commented Apr 24, 2014

Isn't there a way to disable the warning for now?

@seancribbs
Copy link

@evanmcc We're not dialyzer-clean anyway on riak_pb. It hardly matters, except that it might break compilation.

@evanmcc
Copy link
Contributor

evanmcc commented Apr 24, 2014

Right, I was thinking about supressing the warning to get it to compile for
now, to work around erlang's horrible multi-version support.

@kuenishi
Copy link
Contributor Author

In riak_pb, we can do without any error supressing because I added some horrible version-switch :sadpanda:

@kuenishi
Copy link
Contributor Author

I tried passing the same options to Dialyzer but didn't worked.

DIALYZER_APPS = kernel stdlib sasl erts eunit ssl tools crypto \
       inets public_key syntax_tools compiler
DIALYZER_FLAGS = -Wnowarn_deprecated_type
include tools.mk

Which concequnced as follows (with R16B03-1):

...
==> riak-erlang-client (dialyzer)

dialyzer: Unknown dialyzer warning option: nowarn_deprecated_type

So my opinion is to go with ugly version switch by rebar.config.script .

@seancribbs
Copy link

@kuenishi I concur!

@kuenishi
Copy link
Contributor Author

So the current state of pull request already includes version switch script. Any more thing to be done?

@seancribbs seancribbs self-assigned this Apr 30, 2014
@seancribbs
Copy link

@kuenishi Just manual testing.

@seancribbs
Copy link

👍 to merge

kuenishi added a commit that referenced this pull request Apr 30, 2014
compile in 17.0: clients are compiles with various OTP versions
@kuenishi kuenishi merged commit 74831f4 into develop Apr 30, 2014
@kuenishi kuenishi deleted the ku-erlang-17-0 branch April 30, 2014 18:18
@seancribbs seancribbs removed their assignment May 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants