Skip to content

Crystal 0.27.0 support#225

Merged
fridgerator merged 2 commits into
Crecto:masterfrom
drujensen:dj/crystal-0.27.0-support
Nov 3, 2018
Merged

Crystal 0.27.0 support#225
fridgerator merged 2 commits into
Crecto:masterfrom
drujensen:dj/crystal-0.27.0-support

Conversation

@drujensen
Copy link
Copy Markdown
Contributor

@drujensen drujensen commented Nov 3, 2018

Breaking changes for Crystal 0.27.0

@drujensen drujensen changed the title crystal 0.27.0 support Crystal 0.27.0 support Nov 3, 2018
raise Crecto::InvalidType.new("Format validator can only validate strings") unless @instance_hash.fetch(format[:field]).is_a?(String)
val = @instance_hash.fetch(format[:field]).as(String)
raise Crecto::InvalidType.new("Format validator can only validate strings") unless @instance_hash.fetch(format[:field], nil).is_a?(String)
val = @instance_hash.fetch(format[:field], nil).as(String)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the instances of fetch here can be replaced with the regular, non-nilable [], since we're already guarding that the hash has the key on the previous line.

Alternatively, the whole section could be re-written as

if field_value = @instance_hash[format[:field]]?
  raise Crecto::InvalidType.new("Format validator can only validate strings") unless field_value.is_a?(String)
  add_error(format[:field].to_s, "is invalid") if format[:pattern].match(field_value).nil?
end

Crystal's automatic type reduction should ensure that the typing is correct at each point without the need for an .as(String) cast.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the instances of fetch here can be replaced with the regular, non-nilable []

Yes, but I think fetch still looks better IMO.

the whole section could be re-written as

I will leave that exercise to someone else.

@faultyserver
Copy link
Copy Markdown
Contributor

Should probably also bump the .crystal-version file while we're at it (it's still on 0.21.1), or just get rid of it.

I'd vote to keep it and/or add a .tool-versions file to work with the more-generalized asdf version manager

@faultyserver
Copy link
Copy Markdown
Contributor

faultyserver commented Nov 3, 2018

closes #224

@fridgerator fridgerator merged commit 4c0cdfb into Crecto:master Nov 3, 2018
@drujensen drujensen deleted the dj/crystal-0.27.0-support branch November 4, 2018 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants