Skip to content

Clarify & expand service decoration description #16161

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

Closed
wants to merge 5 commits into from
Closed

Clarify & expand service decoration description #16161

wants to merge 5 commits into from

Conversation

gnito-org
Copy link
Contributor

@gnito-org gnito-org commented Nov 26, 2021

I also added an App\Mailer namespace to all code examples.

I also added an `App\\Mailer` namespace to all code examples.
@gnito-org
Copy link
Contributor Author

@javiereguiluz Pinging for a review when you have time. 🙏 😄

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Can you explain why we need the sub Mailer Namespace?

@OskarStark
Copy link
Contributor

@nicolas-grekas would you have some time to review this PR ? Thanks

@gnito-org
Copy link
Contributor Author

Can you explain why we need the sub Mailer Namespace?

@OskarStark Thank you for the review! I added the sub Mailer namespace because that's how it would normally be coded (best practice). We would not put the Mailer.php class in the root of the src folder.

@gnito-org
Copy link
Contributor Author

gnito-org commented Nov 29, 2021

I unintentionally included a commit, what was supposed to be a separate PR, in this PR.

Can it remain in this PR? Honestly, I do not know how to remove it.

The commit was in reponse to #16149.

@gnito-org
Copy link
Contributor Author

I'm closing this PR and redoing it because I forgot to create separate fix branches. That's basically preventing me from creating separate new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants