stedolan / crowbar

Property fuzzing for OCaml
MIT License
183 stars 28 forks source link

New functions ffail and pp_option. Do not quote message in fail. #45

Closed fpottier closed 6 years ago

fpottier commented 6 years ago

pp_option should be uncontroversial.

ffail seems useful; it works like fail, but accepts a format string and extra arguments instead of just a string. One might wish to call it just fail, but that would technically be an incompatible change, because not every string can be safely re-interpreted as a format string.

Finally, I propose changing fail to not quote/escape the message. This looks nicer.

hcarty commented 6 years ago

Should the name be failf to match the naming pattern of functions like printf?

stedolan commented 6 years ago

Looks good! If a new name is chosen, I agree with @hcarty that failf beats ffail. I think that this is how fail should always have worked, but I see how changing it now will break some code.

@yomimono you're probably the person with the most code broken by changing fail (example), what do you think?

yomimono commented 6 years ago

I like what the PR does better (both ffail failf and fail, with backward-compatible fail, unless I misread) and generally like to have convenience functions for shoving strings directly into functions like this one so would prefer to retain fail for that reason, but it's not a huge burden to adapt to that API change if you'd prefer to drop it.

fpottier commented 6 years ago

Hi, I have renamed ffail to failf.

stedolan commented 6 years ago

Thanks!