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

Improve documentation of the binary module #7585

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

Maria-12648430
Copy link
Contributor

  • Many functions in this module are documented as "Same as same-function-with-different-arity(..., Default)", which can be grouped together into a single paragraph which mentions the defaults.
  • The order of the decode_hex, decode_unsigned, encode_hex and encode_unsigned was mingled, ie not in the usual alphabetical order; it might be debatable whether or not opposing functions should be ordered such that they are grouped together (decode_hex, encode_hex, decode_unsigned, encode_unsigned vs decode_hex, decode_unsigned, encode_hex, encode_unsigned), but the current order was off in any case.
  • The documentation for copy explicitly stated that a badarg exception will be raised if N < 0. As the specs already state that N = integer() >= 0, this sentence is IMO redundant.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 23, 2023

CT Test Results

       2 files       91 suites   40m 4s ⏱️
1 935 tests 1 658 ✔️ 277 💤 0
2 237 runs  1 943 ✔️ 294 💤 0

Results for commit 0ac7a38.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@kikofernandez kikofernandez self-assigned this Aug 25, 2023
Copy link
Contributor

@kikofernandez kikofernandez 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 to me. I will wait until Monday in case someone from OTP wants to check this as well.
Merging on Monday

@bjorng bjorng added team:VM Assigned to OTP team VM team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI labels Aug 25, 2023
@kikofernandez kikofernandez merged commit 98b158d into erlang:master Aug 29, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants