phoenixframework / phoenix_html

Building blocks for working with HTML in Phoenix
MIT License
403 stars 220 forks source link

v3.3.2 Breaks dialyzer builds for `input_for/3` #431

Closed elliottneilclark closed 1 year ago

elliottneilclark commented 1 year ago

The spec was deleted while removing the documentation in 9464dfb7a493cd91803e6aefdbe94fb69194c668

Adding the logging on a patch release feels ok, but I don't think breaking mix dialyzer on older Phoenix apps was intended.

dvic commented 1 year ago

What is the dialyzer error you are getting?

josevalim commented 1 year ago

This is intentional because Dialyzer helps people catch the deprecation too, if they miss it otherwise. See #426.

elliottneilclark commented 1 year ago

Breaking builds with a patch release is very unexpected and not a good thing for open-source projects to do.

https://semver.org/#how-should-i-handle-deprecating-functionality

josevalim commented 1 year ago

Yeah, you are right. It should have been in v3.3 with other deprecated functionality, but it fell through the cracks, hence the later patch. But then the whole deprecation should have been on v3.4.x. But if it is deprecated, the specs should go.

elliottneilclark commented 1 year ago

Thanks for the explanation. These things happen and that makes total sense. For me, it shows there's a need for an "integration" test for Phoenix versions. Some example projects(routes with example functionality pulled from unit tests ?) that test to make sure a new version will work with what's been working before.

Rust has https://github.com/rust-lang/crater but something simpler probably does the job.