Skip to content

generate:cest: Adding declare(strict_types=1); and return type void to generated files#6736

Merged
TavoNiievez merged 6 commits intoCodeception:5.2from
ThomasLandauer:patch-8
Jul 10, 2024
Merged

generate:cest: Adding declare(strict_types=1); and return type void to generated files#6736
TavoNiievez merged 6 commits intoCodeception:5.2from
ThomasLandauer:patch-8

Conversation

@ThomasLandauer
Copy link
Copy Markdown
Member

No description provided.

@TavoNiievez
Copy link
Copy Markdown
Member

In the past I was against adding : void at the end of methods because it doesn't add anything relevant to the tests and because it can make the suite tests harder to read.
Strict types were also suggested by me, but it was also not decided to add them.
I even suggested that Cest should be 'final' classes, but again, the simpler the default generated code the better I guess was the reasoning.

@ThomasLandauer
Copy link
Copy Markdown
Member Author

ThomasLandauer commented Feb 18, 2024

Well, you can certainly see it as noise. But that's the way that PHP has been taking over the last few years.

My main argument is: That's the way good PHP code should look nowadays, so we should gently guide people to this direction. And the generated files are just a suggestion - anybody can easily delete the stuff they don't want.

@Naktibalda Naktibalda changed the title Update Cest.php: Adding declare(strict_types=1); and return type void to generated files generate:cest: Adding declare(strict_types=1); and return type void to generated files Feb 19, 2024
@TavoNiievez TavoNiievez changed the base branch from 5.1 to 5.2 July 10, 2024 00:19
Copy link
Copy Markdown
Member

@TavoNiievez TavoNiievez left a comment

Choose a reason for hiding this comment

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

Hi @ThomasLandauer,

In the interest of going forward with this PR I have decided to approve the following changes you argued and I agree with for the next minor version 5.2:

  • declare(strict_types=1);
  • final
  • : void
  • ✅ short comments for each method.

You are of course free to open another PR with the remaining changes:

  • #[Before]. I don't agree with adding this because I think it is adding too much documentation on related methods. Also, the documentation referenced in that link has errors such as "#[Bbefore]".
  • Scenario \$scenario. The use of scenario doesn't seem very common, many people won't need it when writing their tests, so putting it there just to let people know it exists doesn't really seem necessary, since it's unlikely to increase its use.

@ThomasLandauer
Copy link
Copy Markdown
Member Author

OK, this is a step forward.

For the remaining two, I'm undecided myself...

And see Codeception/codeception.github.com#868 for the typos ;-)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants