Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 11, 2023
Merged

Use constructor property promotion in monolog_console.rst code example #18039

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions logging/monolog_console.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,8 @@ The example above could then be rewritten as::

class YourCommand extends Command
{
private $logger;

public function __construct(LoggerInterface $logger)
public function __construct(private LoggerInterface $logger)
{
Comment on lines +48 to 49
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
public function __construct(private LoggerInterface $logger)
{
public function __construct(
private LoggerInterface $logger
){

Copy link
Contributor Author

@timkrase timkrase Mar 10, 2023 •
edited
Loading

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.

Copy link
Contributor

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 ?

Copy link
Member

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

Copy link
Member

@wouterj wouterj Mar 10, 2023 •
edited
Loading

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.

Copy link
Contributor

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

Copy link
Contributor

@OskarStark OskarStark Mar 11, 2023 •
edited
Loading

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

Copy link
Contributor

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

$this->logger = $logger;
}

protected function execute(InputInterface $input, OutputInterface $output)
Expand Down