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

Export oft-used types w/o exposing implementation #1

Closed
wants to merge 1 commit into from

Conversation

mheiber
Copy link
Owner

@mheiber mheiber commented Jan 20, 2021

Export more types that meet these criteria:

  • Documented
  • Convenient for client code. They are already
    used in specs of public functions of their respective
    modules.
  • These types do not expose implementation details.

The types are:

ets.erl:

  • type() :: set | ordered_set | bag | duplicate_bag.

re.erl:

  • mp() :: {re_pattern,_ ,_ ,_ ,_ }.*

supervisor.erl:

  • `restart() :: 'permanent' | 'transient' | 'temporary'
  • mfargs() :: {M :: module(), F :: atom(), A :: [term()] | undefined}.
  • shutdown() :: 'brutal_kill' | timeout().

The reason for this change is to fix the status quo, which is
that teams either must:

  • copy/pasta the types into client code, leading to code bloat and having
    to manually keep the types in sync.
  • OR refer to non-exported types, which will mean either more noise
    or less accuracy from code analysis tools. For example,
    Dialyzer will either warn or treat the types as term().

@mheiber
Copy link
Owner Author

mheiber commented Jan 20, 2021

@max-au does this look ready to open against erlang/otp?

Copy link

@max-au max-au left a comment

Choose a reason for hiding this comment

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

Looks good in general, although exporting type that is seemingly opaque is probably not ideal. Maybe start without re:mp?

lib/stdlib/src/re.erl Outdated Show resolved Hide resolved
Export more types that meet these criteria:

- Documented
- Convenient for client code. They are already
used in specs of public functions of their respective
modules.
- These types do not expose implementation details.

The types are:

ets.erl:
- type() :: set | ordered_set | bag | duplicate_bag.

supervisor.erl:
- `restart() :: 'permanent' | 'transient' | 'temporary'
- `mfargs() :: {M :: module(), F :: atom(), A :: [term()]
   | undefined}.`
- `shutdown() :: 'brutal_kill' | timeout().`

The reason for this change is to fix the status quo, which is
that teams either must:
- copy/pasta the types into client code, leading to code bloat and having
to manually keep the types in sync.
- OR refer to non-exported types, which will mean either more noise
or less accuracy from code analysis tools. For example,
Dialyzer will either warn or treat the types as `term()`.
@mheiber
Copy link
Owner Author

mheiber commented Jan 21, 2021

closing, opening against otp erlang 🤞
erlang#2994

@mheiber mheiber closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants