[Form] moved data trimming logic of TrimListener into StringUtil #14660
Conversation
|
The feature looks strange to me: a StringUtil in the Form component to deal with non-form-related situations. If I'm the only one, then an entry in the CHANGELOG is missing :) |
|
@nicolas-grekas btw, I have just updated CHANGELOG :) |
|
Oh..., I marked deprecation "yes" mistakenly. There is no deprecation in this PR. |
|
ping @webmozart |
| /** | ||
| * This class should not be instantiated. | ||
| */ | ||
| private function __construct() |
There was a problem hiding this comment.
Private methods should go after public methods.
There was a problem hiding this comment.
Not in this case IMHO. Constructor are always first.
There was a problem hiding this comment.
Even before static methods?
There was a problem hiding this comment.
Yes, that's what we are doing in Symfony.
There was a problem hiding this comment.
i think the most important should be first (which are the constructor and public methods). since the constructor is private it doesn't need to be first in this case as this is not what is of interest.
There was a problem hiding this comment.
Actually I think private constructor should be after too, but some util classes' constructor is first. (e.g. classes in Security/Core, FormUtil as well).
|
Thanks @issei-m! Could you rebase this onto 2.8 and fix the issues I mentioned? After that, I'm 👍. |
|
@webmozart Thanks for your good. I've fixed code you mentioned and rebased onto 2.8. |
|
Thanks @issei-m :) |
I introduced this PR at #14146, but I didn't make it on time for 2.7 feature freeze. So I'm opening the new PR for 2.8.
Following is my first introduction:
@dosten
@Tobion
@xabbuh
@stof
@webmozart
Thanks for all your feedback. I have fixed all issues mentioned on #14146. Please re-consider it.