-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Rename MemoryIO to IO::Memory #3563
Conversation
My PR also did a |
|
||
class MemoryIO < IO::Memory | ||
def self.new(*args, **nargs) | ||
{{ puts "Warning: MemoryIO is deprecated and will be removed after 0.20.0, use IO::Memory instead".id }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a line number of the caller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to get that info right now. In the future we could add it, but it will always be the one of the first call site, because later method calls with the same argument types are not re-analyzed
c880cbc
to
0da8a24
Compare
@RX14 Done the a/an replacement, good catch :-) |
@@ -46,7 +46,7 @@ module IO::Buffered | |||
# We first check, after filling the buffer, if the delimiter | |||
# is already in the buffer. In that case it's much faster to create | |||
# a String from a slice of the buffer instead of appending to a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an
here
# raw IO. Optimizations for specific IOs are tested separately | ||
# (for example in buffered_io_spec) | ||
private class SimpleMemoryIO | ||
private class SimpleIO::Memory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be renamed.
@@ -397,8 +397,8 @@ module Crystal | |||
produce_tuple = node.names.first == "T" | |||
when GenericInstanceType | |||
produce_tuple = ((splat_index = path_lookup.splat_index) && | |||
path_lookup.type_vars.keys.index(node.names.first) == splat_index) || | |||
(path_lookup.double_variadic? && path_lookup.type_vars.first_key == node.names.first) | |||
path_lookup.type_vars.keys.index(node.names.first) == splat_index) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this reformatted?
Specs |
I spent quite a bit of time on the old PR, it's not as easy as you would think at first! |
@RX14 Do you want to rebase your PR instead? It will probably be easier :-) |
(well, rebase or do it from scratch, whatever is easier) |
Well you've done a lot with this PR too so why not go ahead with this. |
334a4c4
to
ef0bf43
Compare
MemoryIO is still available but produces a deprecation warning at compile-time, to ease migration.
ef0bf43
to
7c24948
Compare
@@ -3,6 +3,7 @@ | |||
* **(breaking change)** Removed `ifdef` from the language | |||
* **(breaking change)** Removed `PointerIO` | |||
* **(breaking change)** The `body` property of `HTTP::Request` is now an `IO?` (previously it was `String`). Use `request.body.try(&.gets_to_end)` if you need the entire body as a String. | |||
* **(breaking change)** `MemoryIO` has been renamed to `IO::Memory`. The old name can still be used but will produce a compile-time warning. `MemoryIO` will be removed immediately after 0.20.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean removed in 0.21.0 or 0.20.1? This comment seems to imply the latter, which I don't think is what you meant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what I meant. MemoryIO is gone but it's still there to ease migration. I want to remove it as soon as possible. Or we can do it in 0.21.0. In any case we are at 0.x so semver doesn't truly applies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we added a deprecation, we can wait for 0.21 to be released :)
@asterite there are |
@Sija Thanks. It seems they got added after I merged an old PR |
Similar to #3401. MemoryIO is still available but produces a deprecation warning at compile-time, to ease migration.