Skip to content

Fix PSR4 in composer.json, add dump() #490

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
Feb 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
"type": "project",
"description": "Symfony Demo Application",
"autoload": {
"psr-4": { "": "src/" },
"psr-4": {

Choose a reason for hiding this comment

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

whilst this may boost performance, it may be harder for new users don't you think?

Copy link
Member

Choose a reason for hiding this comment

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

But only if you create a ton of bundles in your app ... which by the way is now considered a bad practice in most of the cases!

Choose a reason for hiding this comment

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

Well to be honest I don't even see why you need any bundle here in the first place, but it's not limited to bundles. It highly depends on how you manage your src bundle. I would personally promote more something like having a App namespace for the whole src.

Copy link
Member

Choose a reason for hiding this comment

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

@theofidry the symfony-demo project has a AppBundle namespace for its source, because this is what the Symfony best practices are telling (and the demo is also about showing these best practices).

The extra CodeExplorerBundle is more a project-specific vendor bundle actually (we haven't shared it as a real vendor bundle, because this feature simply does not make sense outside the demo project).

and enw users won't find it harder, as they won't need to change the autoload config of symfony-demo.

"AppBundle\\": "src/AppBundle/",
"CodeExplorerBundle\\": "src/CodeExplorerBundle/"
},
"classmap": [ "app/AppKernel.php", "app/AppCache.php" ]
},
"autoload-dev": {
"psr-4": { "Tests\\": "tests/" }
"psr-4": { "Tests\\": "tests/" },
"files": [ "vendor/symfony/symfony/src/Symfony/Component/VarDumper/Resources/functions/dump.php" ]

Choose a reason for hiding this comment

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

why is this needed? isn't it supposed to already be included from the VarDumper composer.json?

Choose a reason for hiding this comment

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

This is more as to ask for clarifications. I seem to be unable to use the dump function when the app is not bootstrapped sometimes so I took the habit to add a shortcut to make a FQCN call to avoid that problem, but this still looks weird as I would expect to be able to use dump at anytime.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Feb 27, 2017

Choose a reason for hiding this comment

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

When symfony/symfony is required, the dump function is not autoloaded. It is only when symfony/var-dumper directly is.

Choose a reason for hiding this comment

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

Isn't the dump function loaded when Debug::enable() is called?

Copy link
Member

Choose a reason for hiding this comment

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

@Pierstoval no, it is loaded when the DebugBundle is booted. but when running tests, it is common for it not to be booted, making it painful. This is why we added it as a dev autoloading here for convenience.

Choose a reason for hiding this comment

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

Then symfony/symfony-standard#1025 should also be merged, shouldn't it? :)

},
"require": {
"php" : ">=5.5.9",
Expand Down
52 changes: 29 additions & 23 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.