Skip to content
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

Add best practice about the Form type namespace #6059

Merged
merged 1 commit into from
Dec 31, 2015
Merged

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Dec 21, 2015

Q A
Doc fix? no
New docs? yes
Applies to all
Fixed tickets -

/cc @javiereguiluz @weaverryan as you are the main guys behind this guide.

@javiereguiluz
Copy link
Member

I have "strongly mixed feelings":

  • I like this proposal because it looks simple, suited for lots of non-advanced applications and it reduces the perceived complexity (it's just one additional directory, but still).
  • I don't like this proposal because of the .. unless .. clause. I'd like these best practices to be as strict as possible and not depend on external conditions. If your app grows, you have to create the new Type/ dir, update the namespaces, the tests, etc. Some people may ask: why do this refactoring now instead of creating the Type/ dir at the beginning?

For now I'm completely divided 😕

@ogizanagi
Copy link
Contributor

I particularly agree about the second point. I'll add that we're most of the time unlikely to predict what will be in the Form namespace when starting an application, i.e if we'll need types, data-transformers, form extensions, event listeners...

In my experience, as @javiereguiluz pointed out, it's way simpler to deal with the extended Form\Type namespace from the beginning than refactoring everything later. More generally, I'm in favor of reflecting the sub-namespace used by the component itself. It also avoid asking ourself inefficiently where to put/find classes across projects.

To me, the same applies to security voters, auth providers, tokens, etc, but it was changed to keep consistency over the Form\Type vs Form issue.

@weaverryan
Copy link
Member

If we recommended that people put things directly on Form/ and they eventually do have other things, like data transformers or extensions, I don't think that's a problem: simply put those other things in sub-directories and keep the form types at the top-level.

Like I said with voters, there is a cost to making people create a directory just to hold one other directory. Since form "types" are by far the most important and common thing in a Form directory, it makes sense to give it a special status. Also, a "Type" directory does not add more clarity (since it's kind of a terrible world).

👍

@xabbuh xabbuh merged commit 0275bef into symfony:2.3 Dec 31, 2015
xabbuh added a commit that referenced this pull request Dec 31, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Add best practice about the Form type namespace

| Q | A
| --- | ---
| Doc fix? | no
| New docs? | yes
| Applies to | all
| Fixed tickets | -

/cc @javiereguiluz @weaverryan as you are the main guys behind this guide.

Commits
-------

0275bef Add best practice about the Form type namespace
@xabbuh
Copy link
Member

xabbuh commented Dec 31, 2015

I agree with Wouter and Ryan. Please also remember that these best practices are simply guidelines which you do not have to follow. I bet that a lot of development teams out there have there own best practices which differ from what we have in the docs. And that is totally fine if that better fits your needs. But having some guidelines makes it easier for newcomers to structure their application and write good and maintable code (and you will very likely not start developing big applications right from the beginning).

@lyrixx
Copy link
Member

lyrixx commented Feb 27, 2018

Hello.

I'm sorry to reactive this issue/PR/Thread, but I think it's not a food idea to mix Type & Other thing (Extention, EventListener, DataTransformer) in the same folder. Yes we could create a dedicated folder for "other things" but that's weird.

I really agree with @ogizanagi and @javiereguiluz here.

We could say it's not a big deal, but the MakerBundle uses theses best practices to generate code.
And moving each time a new type to the right folder is not really a great DX.

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

Successfully merging this pull request may close these issues.

6 participants