search

sakrejda / protostan

Thin protobuf interface wrapper for Stan
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Makefile should assume user has protoc in path #27

Closed ariddell closed 8 years ago

ariddell commented 8 years ago

Currently the Makefile forces the user to use the included protoc which isn't ideal. The default case will be that the user already has protoc installed (via their operating system package manager).

ariddell commented 8 years ago

This is the last thing to cleanup before pystan-protostan will be tested via travis.

sakrejda commented 8 years ago

I don't think this is a good idea unless there's a way to make it possible/easy to use either. proto2 is still the default and at least the Ubuntu builds didn't include proto3 last I checked.

On Fri, Jan 15, 2016 at 3:47 PM, Allen Riddell notifications@github.com wrote:

This is the last thing to cleanup before pystan-protostan will be tested via travis.

— Reply to this email directly or view it on GitHub https://github.com/sakrejda/protostan/issues/27#issuecomment-172088480.

ariddell commented 8 years ago

I don't think any mature packages (<1% for sure) ship an external tool like protobuf with the package. It would be like packaging the Rcpp source with RStan.

In the short term I think we just need to fix the Makefile so that the user isn't forced to use the protobuf 3 that's a submodule. I'll figure this out. It's just a shell check or two.

sakrejda commented 8 years ago

Agreed, at this point this is hardly a mature package although I know that's a consideration for perching pystan on top of it. If you see a way of doing it so that it's easy to switch go for it. I'm still in the middle of traveling so I won't get to any Stan stuff till sometime next week.

On Fri, Jan 15, 2016 at 8:57 PM, Allen Riddell notifications@github.com wrote:

I don't think any mature packages (<1% for sure) ship an external tool like protobuf with the package. It would be like packaging the Rcpp source with RStan.

In the short term I think we just need to fix the Makefile so that the user isn't forced to use the protobuf 3 that's a submodule. I'll figure this out. It's just a shell check or two.

— Reply to this email directly or view it on GitHub https://github.com/sakrejda/protostan/issues/27#issuecomment-172145792.

ariddell commented 8 years ago

I think this isn't an issue for the present Makefile (which is intended to help us run tests).