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.

Sign up for GitHub

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

Jump to bottom

Finish gen_server event time-outs #9621

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Finish gen_server event time-outs #9621

wants to merge 3 commits into from

Conversation

RaimoNiskanen
Copy link
Contributor

Mainly documentation for the new gen_server event time-outs, continuation of PR #9287.

I also added handling of time infinity, for completeness, because gen_statem's event time-outs handles that, and also since all values allowed for legacy time-outs should work for the new event time-outs.

Test case for Time = infinity is still missing.

Please review the documentation, I am thinking of you; @juhlig and @Maria-12648430...

Related PRs: #9287 and #9615

@RaimoNiskanen RaimoNiskanen added this to the OTP-28.0 milestone Mar 21, 2025
@RaimoNiskanen RaimoNiskanen self-assigned this Mar 21, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 21, 2025 •
edited
Loading

CT Test Results

    2 files     97 suites   1h 13m 3s ⏱️
2 204 tests 2 153 ✅ 51 💤 0 ❌
2 574 runs  2 518 ✅ 56 💤 0 ❌

Results for commit 9542de5.

♻️ 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

@RaimoNiskanen RaimoNiskanen force-pushed the raimo/stdlib/gen_server-finish-event-timeouts branch from 9086027 to 40ada48 Compare March 21, 2025 08:48
juhlig
juhlig suggested changes Mar 21, 2025
View reviewed changes
@juhlig
Copy link
Contributor

juhlig commented Mar 21, 2025

Looks good, just a few nitpicks ;)

@RaimoNiskanen RaimoNiskanen force-pushed the raimo/stdlib/gen_server-finish-event-timeouts branch from 40ada48 to 9619a50 Compare March 21, 2025 09:51
@juhlig
Copy link
Contributor

juhlig commented Mar 21, 2025

Another nitpick, in the test suite, the functions is_in_erlang_hibernate/is_not_in_erlang_hibernate and their failure reasons should be renamed.

@RaimoNiskanen
Copy link
Contributor Author

Good nitpicks!

@RaimoNiskanen
Copy link
Contributor Author

For the record. Test case for Time = infinity is now implemented

juhlig
juhlig approved these changes Mar 21, 2025
View reviewed changes
Copy link
Contributor

@juhlig juhlig left a comment

Choose a reason for hiding this comment

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

LGTM :)

@RaimoNiskanen
Copy link
Contributor Author

This PR:s documentation is now built. Most changes are in the action() type:
https://erlang.org/github-pr/9621/lib/stdlib-6.2.1/doc/html/gen_server.html#t:action/0

@RaimoNiskanen RaimoNiskanen added the testing currently being tested, tag is used by OTP internal CI label Mar 21, 2025
@juhlig
Copy link
Contributor

juhlig commented Mar 26, 2025

@RaimoNiskanen @IngelaAndin what is the status of this PR, I wonder? AFAIK, code freeze is supposed to be ~next week, and while this PR is mainly docs, it does also contain code changes to accomodate for {timeout, infinity, Msg}.

NelsonVides added a commit to NelsonVides/otp that referenced this pull request Mar 27, 2025
@NelsonVides
EDIT: cherry-pick from erlang#9621
2034bcb 
Already done at 54f685b, to be removed
when that PR is merged.
NelsonVides added a commit to NelsonVides/otp that referenced this pull request Mar 27, 2025
@NelsonVides
EDIT: cherry-pick from erlang#9621
27e0379 
Already done at 54f685b, to be removed
when that PR is merged.
@NelsonVides NelsonVides mentioned this pull request Mar 27, 2025
Open
@RaimoNiskanen
Copy link
Contributor Author

I have been away a few days, but expect to merge this today...

@RaimoNiskanen
Document new gen_server time-outs
3537c18
@RaimoNiskanen
Rename hibernation test functions
9542de5 
The test functions are better of not being named
after which specific functions that implements
the hibernation.
@RaimoNiskanen RaimoNiskanen force-pushed the raimo/stdlib/gen_server-finish-event-timeouts branch from a8cb05a to 9542de5 Compare March 28, 2025 08:31
@RaimoNiskanen
Copy link
Contributor Author

Squashed the documentation commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NelsonVides NelsonVides NelsonVides left review comments

@juhlig juhlig juhlig approved these changes

@IngelaAndin IngelaAndin Awaiting requested review from IngelaAndin

At least 1 approving review is required to merge this pull request.

Assignees

@RaimoNiskanen RaimoNiskanen

Labels
documentation feature in progress priority:medium team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Milestone
OTP-28.0
Development

Successfully merging this pull request may close these issues.
3 participants
@RaimoNiskanen @juhlig @NelsonVides