Add config support for protobuffs #150

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants

wb14123 commented Sep 26, 2013

The current protobuffs config only copy "erl_opts" to
"compile_flags". Some other configs such as "imports_dir" is very
importent, too.

Contributor

tuncer commented Sep 27, 2013

Hi @wb14123, can you please document the new option in rebar.config.sample and mirror the same example in rebar_protobuffs_compiler:info?

  1. Shouldn't the new option be named protobuffs_opts (and not protobuff_opts) for consistency?
  2. Shouldn't {compile_flags, ErlOpts} still be passed implicitly?
Add config support for protobuffs
The current protobuffs config only copy "erl_opts" to
"compile_flags". Some other configs such as "imports_dir" are very
importent, too.

wb14123 commented Oct 18, 2013

Hi, @dizzyd, is this patch able to merge? We need this feature in our project and we also want to use the official rebar. Thanks.

Any updates on this one?

Looks like rebar has some issue on finding imported proto files if we dont put them under './' or 'src/' and currently there is no way to specify the real path which holds those proto files from rebar.

Will be glad to see this one goes into the master branch.

Contributor

jaredmorrow commented Dec 31, 2013

@tuncer with the commit added after your comments are you +1 on this change?

Contributor

tuncer commented Jan 1, 2014

+1

Contributor

jaredmorrow commented Jan 1, 2014

I tried this against risk's protobuffs repo and eunit fails with this rebar.

Reproduce:

cp rebar/rebar riak_pb
cd riak_pb
make test

I haven't had a chance to dig in yet, but I'll hold off on merge until I can spend more time on it. /cc @seancribbs

Contributor

seancribbs commented Jan 1, 2014

@jaredmorrow I'll look into it tomorrow.

@seancribbs Any updates on this?

Contributor

seancribbs commented Feb 22, 2014

@UglyTroLL Got pushed to the bottom of the pile, sorry.

wb14123 commented Apr 15, 2014

Hi, what is gong on about this patch?

Contributor

tuncer commented May 29, 2014

What's the status?

Contributor

seancribbs commented May 29, 2014

Unrelated, but this PR on top of the latest rebar causes our plugin in riak_pb not to work.

Contributor

seancribbs commented May 29, 2014

Otherwise I have no objection, the compilation seems to work fine with the additional options.

@tsloughter tsloughter closed this Jun 15, 2014

Contributor

tuncer commented Jun 15, 2014

@tsloughter why did you close this?

Contributor

ferd commented Jun 15, 2014

Hi, this issue was closed in an attempt to do quick basic filtering, with the benediction of rebar project owners. These issues and pull requests are not issues or code we're spitting on, but given the burden of the task and how much code rot may have happened since these were open is unknown from maintainers at this time. All tickets prior to March 2014 were closed and will be reopened on a per-request basis if we see interest from the reporter or contributor, or if some of the issues reported are still valid after the various patches that have made it since they were opened.

This is a fairly brutal first step to help us get a proper understanding of what is still valid or not, but that has been proven efficient in the past. Sorry for the inconvenience, things should go smoother from there on.

wb14123 commented Jun 22, 2014

Hi, I think this feature is useful and the test on rebar itself is passed. Could you find the problem and merge it?

Contributor

tuncer commented Jul 22, 2014

@wb14123, I don't know why this wasn't reopened, but @UglyTroLL submitted the alternative #317. Does #317 work for you?

wb14123 commented Jul 23, 2014

@tuncer Sure, as long as it works.

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