will / crystal-pg

a postgres driver for crystal
BSD 3-Clause "New" or "Revised" License
462 stars 77 forks source link

Crystal 0.34.0 #203

Closed straight-shoota closed 4 years ago

straight-shoota commented 4 years ago

This PR updates for compatibility with newly release Crystal 0.34.0 addressing the breaking changes from crystal-lang/crystal#8885 and crystal-lang/crystal#8424.

bcardiff commented 4 years ago

FYI crystal-db is been updated for 0.34.0 also.

will commented 4 years ago

Cool, thanks. @bcardiff should we also do a cyrstal-db update in the same release as this?

bcardiff commented 4 years ago

I would say so. crystal-db: ~> 0.9.0 will work fine on Crystal 0.34.0 (and some versions before).

While I was checking a change similar to this PR in ecosystem I did, for 0.34.0 the following change. Avoiding the access to os_error.

  begin
    handle_async_frames(read_one_frame(soc.read_char))
  rescue e : IO::Error
    @soc.closed? ? break : raise e
  end

If support for non-latest crystal version is wanted, the following will do.

{% if compare_versions(Crystal::VERSION, "0.34.0-0") > 0 %}
  begin
    handle_async_frames(read_one_frame(soc.read_char))
  rescue e : IO::Error
    @soc.closed? ? break : raise e
  end
{% else %}
  begin
    handle_async_frames(read_one_frame(soc.read_char))
  rescue e : Errno
    e.errno == Errno::EBADF && @soc.closed? ? break : raise e
  rescue e : IO::Error
    # Before 0.34 IO::Error was raised also in some situations
    @soc.closed? ? break : raise e
  end
{% end %}

Is up to you, and I might be missing some subtles errors, but I couldn't find issues with the general catch.

straight-shoota commented 4 years ago

I suppose checking for @soc.closed? on IO::Error should be sufficient.

bcardiff commented 4 years ago

FYI crystal-db 0.9.0 is already available.