<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body>
<div class="moz-cite-prefix">On 3/4/21 7:45 AM, José Valim wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAGnRm4JNFkLk=UWFTeY7udWPm14K1Yii7KNyYRrOWqxyMbFFyg@mail.gmail.com">
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<div dir="ltr">
<div>Thanks for sharing. I have read the proposal and I think it
describes the problem well!</div>
<div><br>
</div>
<div>I have only one minor comment on the proposed solution,
which is this:</div>
<div><br>
</div>
<div>> The new supervisor flag is named <code>shutdown</code>
with possible values <code>normal</code>,
<code>any_significant</code>and <code>all_significant</code>,
with <code>normal</code> being the default.</div>
<div><br>
</div>
<div>I don't like "normal" being the default because now I have
to remember to change two places, the supervisor specification
and the child spec, when configuring a significant child. The
argument for this choice was:</div>
<div><br>
</div>
<div>> With the supervisor <code>shutdown</code> flag set to
<code>normal</code>, the child spec flag
<code>significant</code> is ignored, even if present and set
to <code>true</code>. This is intended
as a safety means to defend against unwanted breaking of old
code.</div>
<div><br>
</div>
<div>I don't think it is possible for old code to break because
there is no old code using significant in a child spec. :)</div>
<div><br>
</div>
<div>Therefore I would propose for the default to be either
any_significant or all_significant (if we want to be
conversative, the latter). If we really think a default of
normal is necessary, then I would propose to at least warn if
the supervisor is normal and a significant child is given, as
that will eventually save someone from debugging why the
significant flag is not working as expected. :)</div>
<div><br>
</div>
<div>I also think #{shutdown => normal} in a supervisor spec
can be confusing, because someone may think it is customizing
the exit reason of the supervisor, which is typically shutdown
(and not normal). If normal is no longer the default, you
could remove the normal option altogether, but if you want to
keep it, perhaps something like ignore_significant is clearer?</div>
</div>
</blockquote>
<br>
I would prefer the normal option remain, but with a different name.
I agree using the name "normal" would be confusing. However, the
option should exist to mark a supervisor as part of a static
supervision hierarchy. There should also be error checking to
ensure child specs with significant set to true cause an error (the
value doesn't get silently ignored). The error can be used as a
return value for supervisor:start_child/2 and would block
significant use where it is considered inappropriate (some
supervisor processes would want to always exist as part of a static
hierarchy).<br>
<br>
I am not sure about a name instead of "normal" for the option.
Alternative name ideas for the option are "default", "none",
"static", "external".<br>
<br>
Thanks,<br>
Michael
</body>
</html>