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

1.6.0 release post #344

Merged
merged 8 commits into from
Oct 7, 2022
Merged

Conversation

beta-ziliani
Copy link
Member

No description provided.

@netlify
Copy link

netlify bot commented Oct 6, 2022

Deploy Preview for crystal-website ready!

Name Link
🔨 Latest commit bdb0f85
🔍 Latest deploy log https://app.netlify.com/sites/crystal-website/deploys/6340324314e423000723a166
😎 Deploy Preview https://deploy-preview-344--crystal-website.netlify.app/2022/10/06/1.6.0-released
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@HertzDevil
Copy link
Contributor

HertzDevil commented Oct 6, 2022

We probably need a section about deprecating suffix-less UInt64 literals (that don't fit into an Int64)

_releases/2022-10-06-1.6.0-released.md Outdated Show resolved Hide resolved

Before 1.6.0 it would print `'a'`, meaning the generic case was considered first. Now it correctly prints `true`.

Several other [improvements](https://github.com/crystal-lang/crystal/pull/10711) where added to the overloading search algorithm, but only under a special flag `-Dpreview_overload_order`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe -Dpreview_overload_order deserves its own blog post once crystal-lang/crystal#11840 is also available. For now it is probably okay to leave the announcement post like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thoughts exactly

Copy link
Member

Choose a reason for hiding this comment

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

A separate blog post seems like a great idea. But then we don't need to mention it here at all.
I would suggest to remove this sentence from the release post. It's not relevant information. We don't want encourage people to use it without further explanations.

_releases/2022-10-06-1.6.0-released.md Outdated Show resolved Hide resolved
_releases/2022-10-06-1.6.0-released.md Outdated Show resolved Hide resolved
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

Other changes that might be relevant:

Comment on lines 20 to 22
## ⚠️ Refactor in console methods

By making some console methods available in Windows, a significant [refactor](https://github.com/crystal-lang/crystal/pull/12352) lead to deprecating three macro methods in `FileDescriptor`: `cooked_from_tc_mode!`, `noecho_from_tc_mode!`, and `raw_from_tc_mode!`. Additionally, methods `#noecho!` and `#raw!` now return `nil`.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is actually a very relevant change for users.
Might make sense to mention it because of the deprecation, but it should be down at the bottom, not the very first item.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, given that we found a shard that broke because of the one below, let me keep them on top.

Copy link
Member

Choose a reason for hiding this comment

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

Which one broke?

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, naqvis/crystar#11

But that was really just a completely inappropriate use of essentially internal API.

I really don't think this needs to be on top.

Comment on lines 24 to 26
## ⚠️ Refactors in `File::Info`

[Another refactor](https://github.com/crystal-lang/crystal/pull/12385) lead to an improvement in the `File` API, which was breaking the abstraction by returning an object intended to be internal. In practice, if for some reason you ended up with an object of type `Crystal::System::FileInfo`, it should now be `File::Info`.
Copy link
Member

@straight-shoota straight-shoota Oct 6, 2022

Choose a reason for hiding this comment

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

Ditto. This is also a very irrelevant change for users. It should not be on the top. And probably a short sentence is enough.


## Performance improvements

[Little improvements](https://github.com/crystal-lang/crystal/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A1.6.0+label%3Aperformance) in the compiler and the stdlib went into this release. We don't have a benchmark for you, but users reported non-negligible improvements in compilation speed.
Copy link
Member

Choose a reason for hiding this comment

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

It's worth mentioning that the improvements are not just about speed but also memory consumption.

Also can we do a quick benchmark to say something like "hello world program compiles 7% faster and uses 10% less memory with 1.6.0"?

Copy link
Member

@straight-shoota straight-shoota Oct 6, 2022

Choose a reason for hiding this comment

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

The increased IO buffer size might be worth an explicit mention (crystal-lang/crystal#12507).

Copy link
Member Author

Choose a reason for hiding this comment

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

Also can we do a quick benchmark to say something like "hello world program compiles 7% faster and uses 10% less memory with 1.6.0"?

Be my guest :-D

Copy link
Member

Choose a reason for hiding this comment

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

Sadly, I couldn't actually verify any noticeable improvement between 1.5.1 and 1.6.0. If anything changed, it's worsening 😢
But that is probably attributed to other changes, of course (not sure what exactly, though). If at all, these performance improvements reduce the impact.
We need an benchmark suite to quantify the performance effects of compiler changes (crystal-lang/crystal#5508).

Comment on lines 67 to 69
## Improvements in the compiler for Windows (native)

[Several improvements](https://github.com/crystal-lang/crystal/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A1.6.0+label%3Aplatform%3Awindows) landed in this release regarding native Windows support. Most notably, [support for building the interpreter](https://github.com/crystal-lang/crystal/pull/12397), [support for libffi](https://github.com/crystal-lang/crystal/pull/12200), and [mutex support](https://github.com/crystal-lang/crystal/pull/12213).
Copy link
Member

Choose a reason for hiding this comment

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

What's non-native Windows support and why is it worth pointing that out?

Suggested change
## Improvements in the compiler for Windows (native)
[Several improvements](https://github.com/crystal-lang/crystal/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A1.6.0+label%3Aplatform%3Awindows) landed in this release regarding native Windows support. Most notably, [support for building the interpreter](https://github.com/crystal-lang/crystal/pull/12397), [support for libffi](https://github.com/crystal-lang/crystal/pull/12200), and [mutex support](https://github.com/crystal-lang/crystal/pull/12213).
## Improvements in the compiler for Windows
[Several improvements](https://github.com/crystal-lang/crystal/pulls?q=is%3Apr+sort%3Aupdated-desc+milestone%3A1.6.0+label%3Aplatform%3Awindows) landed in this release regarding Windows support. Most notably, [support for building the interpreter](https://github.com/crystal-lang/crystal/pull/12397), [support for libffi](https://github.com/crystal-lang/crystal/pull/12200), and [mutex support](https://github.com/crystal-lang/crystal/pull/12213).

foo(Bar1.new)
```

Before 1.6.0 it would print `'a'`, meaning the generic case was considered first. Now it correctly prints `true`.
Copy link
Member

Choose a reason for hiding this comment

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

We should clearly point out that this bug fix involves a hard breaking change that can lead to programs not building any more. And give advise on what to do when you encounter such a problem (probably restructure the overload type restrictions?). I suppose we didn't put a flag to disable this? @HertzDevil

Copy link
Contributor

Choose a reason for hiding this comment

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

A migration guide would be something like:

  • This happens whenever a parameter has a Union restriction in one overload, and the same parameter in another overload has a restriction that isn't a Path or an Underscore.
  • To preserve the old ordering, remove the union overload;
  • To ensure the new ordering applies to older releases, move the union overload above the generic one. (This is because previous Crystal versions consider the two overloads unordered.)

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by "remove the union overload"? Remove the type restriction with the union?

Copy link
Contributor

@HertzDevil HertzDevil Oct 6, 2022

Choose a reason for hiding this comment

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

No, just straight up deleting the foo(x : Bar1 | Bar2) overload because it never matches in existing versions. If we remove the union then the method accepts a different set of arguments.

In reality it could be much more complex because other method parameters could also influence overload ordering.

@beta-ziliani
Copy link
Member Author

Other changes that might be relevant:

* `Dir.pwd` respect `$PWD`: [crystal-lang/crystal#12471](https://github.com/crystal-lang/crystal/pull/12471)

* Improvements in WASM support (?)

* Unicode update: [crystal-lang/crystal#12479](https://github.com/crystal-lang/crystal/pull/12479)

* Support for unicode normalization: [crystal-lang/crystal#11226](https://github.com/crystal-lang/crystal/pull/11226)

I added a small addendum with these, except for the WASM part, since it's only a couple of PRs. I prefer to wait to have something more substantial to report.

@beta-ziliani beta-ziliani merged commit e519b3c into crystal-lang:master Oct 7, 2022
@beta-ziliani beta-ziliani deleted the release/1.6.0 branch October 7, 2022 16:54
@beta-ziliani
Copy link
Member Author

Why Quinton's approval isn't green? He's a crystaller 🤔

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