search

sakrejda / protostan

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

Add test, stan::lang::compile given invalid code #4

Closed ariddell closed 8 years ago

ariddell commented 8 years ago

Wondering if WARN needs to be a separate status. Whoever is using protostan should check to see if there are warning messages (i.e., that messages is not an empty string). Will add some other inline comments.

sakrejda commented 8 years ago

Do you mean that WARN shouldn't be a separate status and that the user should always check for messages? I can see how that makes sense, just checking.

sakrejda commented 8 years ago

I guess it doesn't affect the pull request, just open an issue if my choice of response codes is not clear and you have a different suggestion.

ariddell commented 8 years ago

Correct. I think SUCCESS is a SUCCESS even if there are warnings. Is there any ambiguity if we rely on messages for seeing if there are warnings? Does messages get populated if it's a success with no warnings?

Can one document .proto files? Or does it typically live separately?

sakrejda commented 8 years ago

Yes that makes sense, I guess the tradition is to have something implicit for warnings like having to check messages and the user has to check. My original thought process was that you have to check the state anyway so you might as well check that for SUCCESS/WARN/FAIL rather than having the programming pattern on the API user's end look one way for SUCCESS and another way for WARN. If it's different people might just never check for messages.

I think it's fine to document the proto files, I don't know what is typically done but if I find a good alternative I'll just convert what's in there.