-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Use constructor property promotion in monolog_console.rst code example #18039
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
Conversation
public function __construct(private LoggerInterface $logger) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public function __construct(private LoggerInterface $logger) | |
{ | |
public function __construct( | |
private LoggerInterface $logger | |
){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So
public function __construct(
private Service $service
) {
}
is the formatting that should be used in the doc code examples?
Just asking because before opening the PR I found this disscusion in PR #17204 where you concluded that cpp should be done in the same code style as in the symfony code. And both the examples linked in the PR as well as previous PRs to the docs are formatted like:
public function __construct(private Service $service)
{
}
I'm just curious what's the preferred style for the docs going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I think we keep them in one line if we have only on, but sometimes we keep many on one line too.
@javiereguiluz not sure how we should proceed in general with property promotion? As Code is meant to be executed and docs should be readable, I think we should line break them all the time.
WDYT @symfony/team-symfony-docs ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d vote for a pragmatic middle based on line length: of the code gets too long (around the 70th char?) we break all args onto multiple lines. That’s usually what I do and it’ll keep the code blocks without needing to scroll horizontally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've rejected CPP before: #17204 (comment) & #16087 (comment) But we also merged PRs adding CP... so I'm not sure what our policy is now.
About code style: we always follow Symfony CS (except that we care a bit more about not too long lines). In the Symfony CS, property promotion must always be on a multi line argument list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CCP makes Code examples shorter and less noisy, so we should go for it.
👍 for multiline for readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this PR @nicolas-grekas is going the same direction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed it while merging, thanks
Thank you @timkrase. |
As described in the PR title