Skip to content

Conversation

eksperimental
Copy link
Contributor

First draft.
I need your input where you see the "???"

let me know what you all think

@eksperimental
Copy link
Contributor Author

You can see the live page formatted page here.
https://github.com/eksperimental/elixir/blob/deprecations/lib/elixir/pages/Deprecations.md

@@ -0,0 +1,57 @@
# Deprecations

## Deprecation policy
Copy link
Member

Choose a reason for hiding this comment

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

Shall we call it ## Policy

3. The feature is removed. This can only happen on major releases. This means deprecated features in Elixir v1.x shall only be removed by Elixir v2.x.


## Table of deprecated features: modules, functions, behaviours, protocols, typespecs, attributes and arguments
Copy link
Member

Choose a reason for hiding this comment

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

What about: ## Table of deprecated features or ## Table of deprecations?


Deprecated feature | Deprecated in | Replaced by | Replacement available since
:------------------------ | :------------ | :---------------------- | :---------------------------
`GenEvent` module | [v1.5] | `Supervisor` and `GenServer`; `GenStage`; `:gen_event` | v1.0 (`Supervisor and GenServer`); v1.3 (GenStage); OTP v17.0 (`:gen_server`)
Copy link
Member

Choose a reason for hiding this comment

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

I believe you meant "OTP v17.0 (:gen_event)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

:------------------------ | :------------ | :---------------------- | :---------------------------
`GenEvent` module | [v1.5] | `Supervisor` and `GenServer`; `GenStage`; `:gen_event` | v1.0 (`Supervisor and GenServer`); v1.3 (GenStage); OTP v17.0 (`:gen_server`)
`()` to mean `nil` | [v1.5] | `nil` | v1.0
`@compile {:parse_transform, _}` in `Module` | [v1.5] | __???__ | __???__
Copy link
Member

Choose a reason for hiding this comment

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

??? = None

`Task.find/2` | [v1.3] | Direct message matching | v1.0
Non-map as second arg. in `URI.decode_query/2` | [v1.3] | Use a map | v1.0
`Access` protocol | [v1.1] | `Access` behaviour | v1.1
`?\xHEX` | [v1.1] | `0xHEX` | __???__ (EDITOR'S NOTE: this has been completely deprecated and it shouldn't have been until v2.0)
Copy link
Member

Choose a reason for hiding this comment

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

I believe we had announced it before 1.0 but we forgot to remove it on 1.0 (doing so only on 1.1).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so the 0xHEX is available since v1.0, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yes!

`Access.key/1` | [v1.4] | `Access.key/2` | v1.3
`Behaviour` module | [v1.4] | `@callback` | v1.0
`Enum.uniq/2` | [v1.4] | `Enum.uniq_by/2` | v1.2
`Float.to_char_list/2` | [v1.4] | `:erlang.float_to_list/2` | OTP v17.0
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 refer to OTP as "OTP 17". WIth no v and no minor version because we don't want developers to depend on specific features based on minor OTP versions.

`IEx.Helpers.import_file/2` | [v1.4] | `IEx.Helpers.import_file_if_available/2` | v1.3
`Mix.Utils.underscore/1` | [v1.4] | `Macro.underscore/1` | v1.2
`Mix.Utils.camelize/1` | [v1.4] | `Macro.camelize/1` | v1.2
`Dict` behaviour | [v1.3] | __???__ `Keyword`; `Map` | __???__ v1.0 (Keyword); v1.2 (Map)
Copy link
Member

Choose a reason for hiding this comment

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

This is correct (??? can be removed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CHANGELOG for v1.2 states that states that Dict was soft-deprecated, but a warning is emitted,
936b243#diff-98fc93be50711b2d5c492ef04d076f52R48

should we update CHANGELOG?

Copy link
Member

@josevalim josevalim Feb 25, 2017

Choose a reason for hiding this comment

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

@eksperimental we warned only for those using Dict to implement their own dicts. The API itself was OK until later. So there are two deprecations: the Dict behaviour and the Dict API. The Dict behaviour has no replacement, as there is no longer a Dict thingy to implement, the Dict API is the one you mentioned above. So I would recommend changing "Dict behaviour" by "Dict module" on this line.

Copy link
Contributor Author

@eksperimental eksperimental Feb 25, 2017

Choose a reason for hiding this comment

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

exactly. so what I did was to add bellow

`Dict` behaviour          | [v1.2]        | *None*                  | *None*

still feels weird not to suggest an alternative

`Float.to_char_list/1` | [v1.5] | `Float.to_charlist/1` | v1.3
`Integer.to_char_list/1` | [v1.5] | `Integer.to_charlist/1` | v1.3
`Kernel.to_char_list/1` | [v1.5] | `Kernel.to_charlist/1` | v1.3
`String.to_char_list/1` | [v1.5] | `String.to_charlist/1` | v1.3
Copy link
Member

Choose a reason for hiding this comment

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

There's the :charlists option in Inspect.Opts as well I believe.

`Keyword.size/1` | [v1.3] | `Kernel.length/1` | v1.0
`Map.size/1` | [v1.3] | `Kernel.map_size/1` | v1.0
`/r` option in `Regex` | [v1.3] | `/U` | v1.1
`Set` behaviour | [v1.3] | __???__ | __???__
Copy link
Member

Choose a reason for hiding this comment

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

There's no replacement for the behaviour, now you should just use MapSet.

`Mix.Utils.camelize/1` | [v1.4] | `Macro.camelize/1` | v1.2
`Dict` behaviour | [v1.3] | __???__ `Keyword`; `Map` | __???__ v1.0 (Keyword); v1.2 (Map)
`\x{X*}` inside strings/sigils/charlists | [v1.3] | `\uXXXX`; `\u{X*}` | v1.1
Map/dictionary as second arg. in `Enum.group_by/3` | [v1.3] | *n/a* | *n/a*
Copy link
Member

Choose a reason for hiding this comment

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

in Enum.group_by/2. I believe we didn't have group_by/3 by then.

Copy link
Member

Choose a reason for hiding this comment

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

What about using None instead of n/a which may not be clear in intent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josevalim We have had Enum.group_by/3 since v1.0

Non-map as second arg. in `URI.decode_query/2` | [v1.3] | Use a map | v1.0
`Access` protocol | [v1.1] | `Access` behaviour | v1.1
`?\xHEX` | [v1.1] | `0xHEX` | __???__ (EDITOR'S NOTE: this has been completely deprecated and it shouldn't have been until v2.0)
`as: true \| false` in `alias/2` and `require/2` | [v1.1] | __???__ *n/a* | *n/a*
Copy link
Member

Choose a reason for hiding this comment

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

??? = None

@josevalim
Copy link
Member

This looks great, I have added some feedback. :)

Copy link
Contributor Author

@eksperimental eksperimental left a comment

Choose a reason for hiding this comment

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

All suggestions have been implemented.
PR is ready for a new review,
I have added comments in the rows introduced, for easier reviewing. thank you.

`String.valid_character?/1` | [v1.3] | `String.valid?/1` | v1.0
`Task.find/2` | [v1.3] | Direct message matching | v1.0
Non-map as second arg. in `URI.decode_query/2` | [v1.3] | Use a map | v1.0
`Dict` behaviour | [v1.2] | *None* | *None*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been added, please review.

`Mix.Utils.underscore/1` | [v1.4] | `Macro.underscore/1` | v1.2
`Mix.Utils.camelize/1` | [v1.4] | `Macro.camelize/1` | v1.2
`Dict` behaviour | [v1.3] | __???__ `Keyword`; `Map` | __???__ v1.0 (Keyword); v1.2 (Map)
`Dict` module | [v1.3] | `Keyword`; `Map` | v1.0 (`Keyword`); v1.2 (`Map`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been changed from "behaviour" to "module". please review.

`Float.to_string/2` | [v1.4] | `:erlang.float_to_binary/2` | OTP v17.0
`Float.to_char_list/2` | [v1.4] | `:erlang.float_to_list/2` | OTP 17
`Float.to_string/2` | [v1.4] | `:erlang.float_to_binary/2` | OTP 17
`HashDict` module | [v1.4] | `Map` | v1.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not documented. And it has been added, please review.

`Float.to_char_list/2` | [v1.4] | `:erlang.float_to_list/2` | OTP 17
`Float.to_string/2` | [v1.4] | `:erlang.float_to_binary/2` | OTP 17
`HashDict` module | [v1.4] | `Map` | v1.2
`HashSet` module | [v1.4] | `MapSet` | v1.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not documented. And it has been added, please review.

`Kernel.to_char_list/1` | [v1.5] | `Kernel.to_charlist/1` | v1.3
`String.to_char_list/1` | [v1.5] | `String.to_charlist/1` | v1.3
`t:char_list/0` typespec | [v1.5] | `t:charlist/0` typespec | v1.3
`:char_lists` key in `t:Inspect.Opts.t/0` type | [v1.5] | `:charlists` | v1.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not documented. And it has been added, please review.

`String.to_char_list/1` | [v1.5] | `String.to_charlist/1` | v1.3
`t:char_list/0` typespec | [v1.5] | `t:charlist/0` typespec | v1.3
`:char_lists` key in `t:Inspect.Opts.t/0` type | [v1.5] | `:charlists` | v1.3
`:as_char_lists` value in `t:Inspect.Opts.t/0` type | [v1.5] | `:as_charlists` | v1.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was not documented. And it has been added, please review.

Copy link
Contributor Author

@eksperimental eksperimental left a comment

Choose a reason for hiding this comment

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

Should we add links to external docs and pages?

:------------------------ | :------------ | :---------------------- | :---------------------------
`Atom.to_char_list/1` | [v1.5] | `Atom.to_charlist/1` | v1.3
`Float.to_char_list/1` | [v1.5] | `Float.to_charlist/1` | v1.3
`GenEvent` module | [v1.5] | `Supervisor` and `GenServer`; `GenStage`; `:gen_event` | v1.0 (`Supervisor` and `GenServer`); v1.3 (`GenStage`); OTP 17 (`:gen_event`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we link to GenStage and Erlang docs for :gen_event ?

Copy link
Member

Choose a reason for hiding this comment

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

I would say yes to these two because they are external to Elixir and it could help discoverability to link them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so for GenStage, should we link to Github repo or hex package, or hexdocs page?

Copy link
Member

@whatyouhide whatyouhide Feb 27, 2017

Choose a reason for hiding this comment

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

I think the right place is hexdocs.pm

Edit: maybe the right place is Hex though. Because it links to GitHub, to the docs, and can be considered as a general hub. Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It looks like the Hex package page is the place to link to.

`HashSet` module | [v1.4] | `MapSet` | v1.1
`Set` module | [v1.4] | `MapSet` | v1.1
`Stream.uniq/2` | [v1.4] | `Stream.uniq_by/2` | v1.2
`IEx.Helpers.import_file/2` | [v1.4] | `IEx.Helpers.import_file_if_available/1` | v1.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we link to the docs page in hexdocs.pm?

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 we should link all of these, they're quite a lot. :| Did you mean linking all of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope. I mean those for other Elixir apps, such as IEx, which do not get linked automatically.

`Kernel.to_char_list/1` | [v1.5] | `Kernel.to_charlist/1` | v1.3
`String.to_char_list/1` | [v1.5] | `String.to_charlist/1` | v1.3
`()` to mean `nil` | [v1.5] | `nil` | v1.0
`:as_char_lists` value in `t:Inspect.Opts.t/0` type | [v1.5] | `:as_charlists` | v1.3
Copy link
Member

Choose a reason for hiding this comment

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

:as_char_lists value -> :as_char_lists field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here, :as_char_lists is a value, not a key/field

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

Ops, missed that. Sorry!

`String.to_char_list/1` | [v1.5] | `String.to_charlist/1` | v1.3
`()` to mean `nil` | [v1.5] | `nil` | v1.0
`:as_char_lists` value in `t:Inspect.Opts.t/0` type | [v1.5] | `:as_charlists` | v1.3
`:char_lists` key in `t:Inspect.Opts.t/0` type | [v1.5] | `:charlists` | v1.3
Copy link
Member

Choose a reason for hiding this comment

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

key ->field

`()` to mean `nil` | [v1.5] | `nil` | v1.0
`:as_char_lists` value in `t:Inspect.Opts.t/0` type | [v1.5] | `:as_charlists` | v1.3
`:char_lists` key in `t:Inspect.Opts.t/0` type | [v1.5] | `:charlists` | v1.3
`char_list/0` type | [v1.5] | `charlist/0` type | v1.3
Copy link
Member

Choose a reason for hiding this comment

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

Should this be linked with t:?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since it's built in, it doesn't link to anything and it leaves the t:.
I think it's an ExDoc bug. While it doesn't link, it should still remove the t:

Copy link
Member

Choose a reason for hiding this comment

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

👍

@eksperimental
Copy link
Contributor Author

@josevalim @whatyouhide could you please review the lines commented in #5808 (review)
the code is outdated, but the lines are still valid. it is just rows that has been introduced after the first commit

`Dict` module | [v1.3] | `Keyword`; `Map` | v1.0 (`Keyword`); v1.2 (`Map`)
`Keyword.size/1` | [v1.3] | `Kernel.length/1` | v1.0
`Map.size/1` | [v1.3] | `Kernel.map_size/1` | v1.0
`Set` behaviour | [v1.3] | *None* | *None*
Copy link
Member

Choose a reason for hiding this comment

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

Theoretically the replacement for Set behaviour is to use MapSet directly.

`Set` behaviour | [v1.3] | *None* | *None*
`String.valid_character?/1` | [v1.3] | `String.valid?/1` | v1.0
`Task.find/2` | [v1.3] | Direct message matching | v1.0
`:append_first` option in `Kernel.defdelegate/2` | [v1.3] | *None* | *None*
Copy link
Member

Choose a reason for hiding this comment

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

Theoretically the replacement is defining the function directly.

`Access` protocol | [v1.1] | `Access` behaviour | v1.1
`as: true \| false` in `alias/2` and `require/2` | [v1.1] | *None* | *None*
`?\xHEX` | [v1.1] | `0xHEX` | v1.0
Empty string in `String.starts_with?/2`, `String.ends_with?/2`, `String.contains?/2`. *__NOTE__: Feature made back available in v1.3* | [v1.1] to [v1.2] | *None* | *None*
Copy link
Member

Choose a reason for hiding this comment

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

Theoretically the replacement is to explicitly check for "" for before hand.

@josevalim
Copy link
Member

@eksperimental do we plan to include syntax deprecations? Such as local_fun_as_variable and fn -> end?

@eksperimental
Copy link
Contributor Author

yes, I think we should. Anything that emits a warning should be listed here,

what is the fn -> end deprecation?

@josevalim
Copy link
Member

iex(1)> fn -> end
warning: an expression is always required on the right side of ->. Please provide a value after ->
  iex:1

@eksperimental
Copy link
Contributor Author

eksperimental commented Mar 3, 2017

I have addressed all the suggestions,
Please have a look at the "None"s and let me know if there is an alternative.

Also I am thinking that there are more syntax deprecations to add. What we could do is to split the table in two. One for Syntax and one for the rest.
But it would leave this as it is for now, merge it and then work on those in a second PR

WDYGT?

@josevalim
Copy link
Member

If we are worried about the table size we can also break the table per application. Although that may not help much.

`\x{X*}` inside strings/sigils/charlists | [v1.3] | `\uXXXX`; `\u{X*}` | v1.1
Map or dictionary as second argument in `Enum.group_by/3` | [v1.3] | Pass a function | v1.3
Non-map as second argument in `URI.decode_query/2` | [v1.3] | Use a map | v1.0
`Dict` behaviour | [v1.2] | *None* | *None*
Copy link
Member

Choose a reason for hiding this comment

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

The replacement is the Map data structure (similar to Set being the MapSet data structure).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@josevalim
Copy link
Member

@eksperimental I gave another pass and found only one place where we could change None to something else. I have added it as a comment. ❤️

@eksperimental eksperimental changed the title WIP: Add Deprecations page Add Deprecations page Mar 4, 2017
@eksperimental
Copy link
Contributor Author

I have merged the last two columns.
This is ready for a final review/merging.
thank you everyone for your input

@whatyouhide whatyouhide merged commit 4020b7e into elixir-lang:master Mar 4, 2017
@whatyouhide
Copy link
Member

Thanks! 💟

@eksperimental eksperimental deleted the deprecations branch March 4, 2017 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants