Misleading method parameter comment for $reply

Updated the method attribute signature comment with a more accurate description. Please review my MR.

In Drupal core changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies → . Also mentioned on the version → section of the list of issue fields documentation. Thanks.

Applied the suggestion. Thanks.

Seems straight forward.

@peter majmesku fyi you shouldn't RTBC your own stuff.

> @peter majmesku fyi you shouldn't RTBC your own stuff.

Well, I RTBC'd your change.

I cannot add multiple email addresses in reply-to. It says invalid

@java008 We don't generally handle support requests in the Drupal.org core issue queue, so you will want to look elsewhere (like the Drupal community Slack) to get support for your issue. Thanks!

Thanks for working on this. I recommend some additional documentation on the format of the paramenter. We don't need to repeat the full $to docs, but we should at least say it has the same format as that parameter. Thanks!

> We don't need to repeat the full $to docs, but we should at least say it has the same format as that parameter. Thanks!

I've added this information. Please review. Thanks!

Thank you for your work to improve Drupal's documentation! I added a suggestion to the MR.

Thanks, I reviewed the edit and applied it.

There was a known test failure in that was fixed in HEAD, so I rebased the branch.

FWIW, the change looks good to me now and I'm willing to RTBC it.

Thanks for working on this.

It seems like the same parameter with a same docs (Optional email address to be used to answer.) is also in core/lib/Drupal/Core/Mail/MailManager.php (see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...).

\Drupal\Core\Mail\MailManager::mail() passes the $reply parameter directly to the \Drupal\Core\Mail\MailManager::doMail(), so I suggest that we fix both occurrences here in this issue, as these are very closely related.

I briefly looked at the tests and it does not seems like that \Drupal\Tests\system\Kernel\Mail\MailTest::testFromAndReplyToHeader() is testing multiple emails in the reply-to header. So I suggest we extend the test and test the support for multiple email addresses, since we are putting it to the docs.

Thanks!

@poker I do not like this issue becoming another never ending story with endless re-openings for 1 line of comment in a class method. Could you please open a new issue for the other class, so we can merge this here? You could provide a MR in your new issue, so this comes on track.

My intention was not to hold this issue, but part of the RTBC issues review process → is evaluating the scope of the issue. I definitely think that changing the documentation for a same parameter in two functions directly related should be done in one issue, not in two issues, because it will create unnecessary work for maintainers a reviewers, see: https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... →

If we take a look at \Drupal\Core\Mail\MailManagerInterface::mail() and \Drupal\Core\Mail\MailManager::doMail(), the documentation is mostly the same for each parameter. If we update only one of these functions, we will end up in inconsistent state.

Regarding a missing test - yes, there is a mention in the IS, that multiple addresses in the reply-to header works, but there is no proof for that (and also there is a comment #11, which states the opposite). Ideally, there should be either a test for this (which I have not found) or at least a confirmation (for example link to the documentation), that all implementations of \Drupal\Core\Mail\MailInterface::mail() supports it (at least the two implementations, which are supported by the core - PHP mail and Symfony mailer).

Thanks!

@poker10 Thanks for your hint and the links to the docs. I have updated the doc comment for core/lib/Drupal/Core/Mail/MailManager.php, too.