Skip to content

Add missing types in code templates #1109

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

Merged
merged 1 commit into from
May 3, 2022
Merged

Conversation

nicolas-grekas
Copy link
Member

No description provided.

// ->getQuery()
// ->getResult()
// ;
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

sticking to a consistent CS for examples

{
// ...
}

public static function getSubscribedEvents()
public static function getSubscribedEvents(): array
Copy link
Member Author

Choose a reason for hiding this comment

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

triggers a deprecation without the return type

@@ -11,15 +11,15 @@ class <?= $class_name ?> extends Voter
public const EDIT = 'POST_EDIT';
public const VIEW = 'POST_VIEW';

protected function supports(<?= $use_type_hints ? 'string ' : null ?>$attribute, $subject): bool
protected function supports(string $attribute, $subject): bool
Copy link
Member Author

Choose a reason for hiding this comment

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

Kernel is at 5.4 minimum already

{
// TODO: return your encoded data
return '';
}

public function supportsEncoding($format): bool
public function supportsEncoding(string $format, array $context = []): bool
Copy link
Member Author

Choose a reason for hiding this comment

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

not adding $context is deprecated

{
$data = $this->normalizer->normalize($object, $format, $context);

// Here: add, edit, or delete some data
// TODO: add, edit, or delete some data
Copy link
Member Author

Choose a reason for hiding this comment

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

consistency

*/
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
Copy link
Member Author

Choose a reason for hiding this comment

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

because we can, isn't it?

@nicolas-grekas nicolas-grekas force-pushed the types branch 2 times, most recently from ebb2c18 to f53c858 Compare May 2, 2022 17:07
Copy link
Collaborator

@jrushlow jrushlow left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks @nicolas-grekas quick comment on the test return types.

/cc @weaverryan

@jrushlow jrushlow added Feature New Feature Status: Reviewed Has been reviewed by a maintainer labels May 3, 2022
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

This is really nice 👍

@weaverryan
Copy link
Member

Thanks Nicolas!

@weaverryan weaverryan merged commit 33a18db into symfony:main May 3, 2022
@jrushlow jrushlow mentioned this pull request May 4, 2022
@nicolas-grekas nicolas-grekas deleted the types branch May 13, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants