Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return Int64 from IO#write (Crystal 0.35) #3

Merged
merged 3 commits into from
Jun 12, 2020

Conversation

jgillich
Copy link
Contributor

See crystal-lang/crystal#9454

Automatic formatting also did some stuff, I can get rid of that if you don't like it

@m-o-e
Copy link
Member

m-o-e commented Jun 11, 2020

@jgillich Thanks!

Looks good to me. I'm surprised it actually still compiles on 0.33.0 (that's what CircleCI uses).
Could you update the .circleci/config.yml to 0.35? Then we can merge and release.

@@ -2,17 +2,23 @@ require "openssl"

class Rucksack
class NotFound < Exception; end

Copy link
Member

Choose a reason for hiding this comment

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

Not a fan of these blank lines, but if that's what crystal format
wants then, oh well, probably easier to keep them than to argue
with it every time... ;)

@jgillich
Copy link
Contributor Author

@m-o-e Done

@m-o-e
Copy link
Member

m-o-e commented Jun 11, 2020

@jgillich
Ah! Now there's a warning (see make test output):
Warning: Deprecated OpenSSL::Digest#digest. Use final instead.

Wanna give that a tackle, too? 😬

@jgillich
Copy link
Contributor Author

Also renamed the method on Checksummer, not sure if that makes sense or not 🤔

@m-o-e
Copy link
Member

m-o-e commented Jun 12, 2020

Yes it does.

Thanks a lot, looks great! 🙇

Will merge, add --error-on-warnings to the test suite
(noticed only now that it's missing) and cut a new release. :)

@m-o-e m-o-e merged commit 6c6b1fd into busyloop:master Jun 12, 2020
@jgillich
Copy link
Contributor Author

jgillich commented Jun 12, 2020

Ah, you should also set crystal: ">=0.35.0" in the shard.yml.

@m-o-e
Copy link
Member

m-o-e commented Jun 12, 2020

Yes, I've updated it. Release is out now, please let me know in case you
notice any problems. And thanks again for the help! 👍

@jgillich
Copy link
Contributor Author

Will do, thanks!

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.

2 participants