Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cleanup: use final classes #1409

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Conversation

COil
Copy link
Contributor

@COil COil commented Mar 18, 2023

Hello, these classes could be final right? (except the entities)

(just done one example, will do the other if it is OK)

@derrabus
Copy link
Member

In my apps, I usually declare all classes final unless I have a good reason not to. I wouldn't mind if the demo app followed this rule as well. 🙂

So 👍🏻 from my side.

@COil
Copy link
Contributor Author

COil commented Mar 19, 2023

The only problem I see is that the maker bundle doesn't put the final keyword on controllers.

@COil COil force-pushed the chore/final-classes branch from 3421c18 to d24a930 Compare March 20, 2023 22:36
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.

The only problem I see is that the maker bundle doesn't put the final keyword on controllers.

@weaverryan should we use final for controllers in maker bundle?

@COil COil marked this pull request as draft March 21, 2023 07:58
@COil COil force-pushed the chore/final-classes branch from d24a930 to 180fa15 Compare March 21, 2023 19:20
@GromNaN
Copy link
Member

GromNaN commented Mar 21, 2023

There are good reasons to use final by default. Can we explain with a comment somewhere is this project?

@COil COil marked this pull request as ready for review March 21, 2023 21:10
@javiereguiluz
Copy link
Member

This practice is very common in Symfony apps, so let's merge this. Thanks Loïc!

@javiereguiluz javiereguiluz merged commit 83f8308 into symfony:main Mar 22, 2023
@COil COil deleted the chore/final-classes branch March 30, 2023 20:22
@jrushlow
Copy link

jrushlow commented May 6, 2024

The only problem I see is that the maker bundle doesn't put the final keyword on controllers.

@weaverryan should we use final for controllers in maker bundle?

@OskarStark symfony/maker-bundle#1539

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants