Skip to content

Fix supervisor:terminate/2 spec and add some local specs#10772

Open
erszcz wants to merge 1 commit intoerlang:masterfrom
erszcz:radek.szymczyszyn/fix-supervisor-terminate-spec
Open

Fix supervisor:terminate/2 spec and add some local specs#10772
erszcz wants to merge 1 commit intoerlang:masterfrom
erszcz:radek.szymczyszyn/fix-supervisor-terminate-spec

Conversation

@erszcz
Copy link
Contributor

@erszcz erszcz commented Feb 25, 2026

terminate_dynamic_children/1 returns ok from maps:foreach(...), which matches the spec, but terminate_children/2, itself lacking a spec, returns NChildren returned by children_map/2.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 25, 2026

CT Test Results

    2 files    100 suites   1h 9m 23s ⏱️
2 288 tests 2 237 ✅ 51 💤 0 ❌
2 706 runs  2 650 ✅ 56 💤 0 ❌

Results for commit cb2e967.

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

@erszcz erszcz force-pushed the radek.szymczyszyn/fix-supervisor-terminate-spec branch from ca9c330 to ece55bb Compare February 26, 2026 00:22
@IngelaAndin IngelaAndin added the team:PS Assigned to OTP team PS label Mar 2, 2026
@IngelaAndin IngelaAndin self-assigned this Mar 3, 2026
@erszcz erszcz force-pushed the radek.szymczyszyn/fix-supervisor-terminate-spec branch from 36afea4 to 8cb8445 Compare March 9, 2026 18:24
@IngelaAndin IngelaAndin added the testing currently being tested, tag is used by OTP internal CI label Mar 10, 2026
Comment on lines +1561 to +1562
-spec terminate_children(children(), SupName) -> children() when
SupName :: {local, atom()} | {global, term()} | {pid(), module()}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-spec terminate_children(children(), SupName) -> children() when
SupName :: {local, atom()} | {global, term()} | {pid(), module()}.
-spec terminate_children(children(), SupName) -> children() when
SupName :: {'local', atom()} | {'global', term()} | {'via', module(), term()} | {pid(), module()}.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that would adhere to the style around

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point that SupName can't really be a {pid(), module()} anymore! Apparently, there's already a type defined for it. Pushed a change to just use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I didn't say that! It will be {pid(), module()} if the supervisor is started by start_link/2, ie unregistered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, indeed, my bad! I hope I got it right this time 🤞

@erszcz erszcz force-pushed the radek.szymczyszyn/fix-supervisor-terminate-spec branch from 8cb8445 to 670d6fc Compare March 12, 2026 17:38
@erszcz erszcz requested a review from Maria-12648430 March 12, 2026 17:39
@erszcz erszcz force-pushed the radek.szymczyszyn/fix-supervisor-terminate-spec branch 3 times, most recently from 7f820a7 to da18f47 Compare March 17, 2026 17:47
@erszcz erszcz changed the title Fix supervisor:terminate/2 spec, add terminate_children/2 spec Fix supervisor:terminate/2 spec and add some local specs Mar 17, 2026
`terminate_dynamic_children/1` returns `ok` from `maps:foreach(...)`,
which matches the spec, but `terminate_children/2`, itself lacking a spec,
returns `NChildren` returned by `children_map/2`.
@erszcz erszcz force-pushed the radek.szymczyszyn/fix-supervisor-terminate-spec branch from 2739b2e to cb2e967 Compare March 18, 2026 20:48
@IngelaAndin IngelaAndin added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Mar 19, 2026
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 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