Skip to content

Update to crystal 0.25.0#826

Closed
bcardiff wants to merge 14 commits into
amberframework:masterfrom
bcardiff:crystal/0.25.0
Closed

Update to crystal 0.25.0#826
bcardiff wants to merge 14 commits into
amberframework:masterfrom
bcardiff:crystal/0.25.0

Conversation

@bcardiff
Copy link
Copy Markdown
Contributor

@bcardiff bcardiff commented May 28, 2018

This PR updates to upcoming breaking changes in crystal 0.25.0
Note that this also depends on the following PR been available as a formal release to update the shards.yml

Fixes #847

bcardiff and others added 2 commits May 24, 2018 19:35
Use new File::Permissions and File.info
Update to JSON::Any API
@drujensen
Copy link
Copy Markdown
Member

@bcardiff Any idea when the docker image will be created for 0.25.0?

https://hub.docker.com/r/crystallang/crystal/tags/

@robacarp
Copy link
Copy Markdown
Member

@bcardiff thank you for doing this, and especially for documenting the needed changes that are prerequisites

@bcardiff
Copy link
Copy Markdown
Contributor Author

@drujensen Ideally during next week the former release should be made. If you need, you can use the crystallang/crystal:nightly docker image.

@ilovezfs
Copy link
Copy Markdown

In Homebrew, I've shipped crystal-lang 0.25.0, which has broken the amber formula. It would be great if we could unbreak that as soon as possible.

@faustinoaq
Copy link
Copy Markdown
Contributor

@ilovezfs Yeah, no problem, I'm already fixing dependencies, we should be ready on next days 👍

@faustinoaq
Copy link
Copy Markdown
Contributor

@bcardiff I tried to update dependencies, although seems lib/pg (https://github.com/will/crystal-pg) also needs to fix breaking-changes 😅

in lib/pg/src/pg/decoder.cr:4: undefined constant JSON::Type

Ref: https://travis-ci.org/amberframework/amber/jobs/392749736#L716

/cc @will

@will
Copy link
Copy Markdown

will commented Jun 15, 2018

pg v0.15.0 was released just now and should fix this

@faustinoaq
Copy link
Copy Markdown
Contributor

@will Thank you!

I already sent a PR to Granite (1), Crecto (2), and Jennifer (3) repositories

(1) amberframework/granite#228
(2) Crecto/crecto#165
(3) imdrasil/jennifer.cr#143

@faustinoaq
Copy link
Copy Markdown
Contributor

Oh, seems liquid also need some updates to fix breaking changes 😅

^^ amberframework/liquid.cr#33

@ilovezfs
Copy link
Copy Markdown

I guess CI needs to be retriggered now that liquid.cr was fixed?

@faustinoaq
Copy link
Copy Markdown
Contributor

Hi, liquid.cr is fine 👍

Now we're getting errors with amber router /cc @robacarp


in spec/amber/router/cookies_spec.cr:27: undefined method 'rfc1123_date' for HTTP:Module

Ref: https://travis-ci.org/amberframework/amber/jobs/393041252#L680

@felixbuenemann
Copy link
Copy Markdown

@faustinoaq I think crystal 0.25.0 lacks direct support for rfc1123, only rcf2822 is exposed.

You can get it by implementing your own formatter function:

def rfc1123_date(time)
  String.build do |io|
    formatter = Time::Format::Formatter.new(time.to_utc, io)
    formatter.rfc_2822(true, true)
    io
  end
end

time = Time.utc(2017, 6, 7, 9)

puts time.to_rfc2822
=> "Wed, 7 Jun 2017 09:00:00 +0000"

puts rfc1123_date(time)
=> "Wed, 07 Jun 2017 09:00:00 GMT"

That's probably an oversight of crystal-lang/crystal#5123.

@bcardiff
Copy link
Copy Markdown
Contributor Author

bcardiff commented Jun 16, 2018 via email

@felixbuenemann
Copy link
Copy Markdown

@bcardiff Thanks, I missed that. I think you meant to link to HTTP.format_time(time):

https://crystal-lang.org/api/0.25.0/HTTP.html#format_time%28time%3ATime%29%3AString-class-method

So replacing HTTP.rfc1123_date with HTTP.format_time should work.

@ilovezfs
Copy link
Copy Markdown

@faustinoaq what's the status here?

@robacarp
Copy link
Copy Markdown
Member

@ilovezfs #857 is the pull addressing this issue.

Unfortunately we're in a dependency-bitrot hell.

@robacarp robacarp closed this Jun 18, 2018
@felixbuenemann
Copy link
Copy Markdown

@robacarp Why was this issue closed if #857 is still open?

@robacarp
Copy link
Copy Markdown
Member

@felixbuenemann this isn't an issue, it's a pull request which has been replaced.

@felixbuenemann
Copy link
Copy Markdown

Oh my, sorry for the confusion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants