Quick review of 'create framework' tutorial #5570
Conversation
|
|
||
| # After | ||
| /hello/Fabien | ||
| instead of relying on a query string (like ``/hello/Fabien`` instead of |
There was a problem hiding this comment.
I'm not sure if the new version is much better than the old one. The repeated instead of is confusing to me.
There was a problem hiding this comment.
I tweaked this here to avoid the 2 instead of - sha: 3640b2a
|
You could also add the missing use statement for the HttpKernel component (see #5556 (comment)). |
|
That was the original reason for doing this PR, but it turns out it isn't missing. It's included in a previous chapter or code example. |
|
I see, but then we should imo add |
|
Then we need to do it for 99% of the examples (as it's a tutorial, all code examples are an improvement of the previous shown example) |
|
Hm, you're right. I think we can keep it as is then. |
| bootstrap="vendor/autoload.php" | ||
| <phpunit | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.1/phpunit.xsd" |
There was a problem hiding this comment.
I suggest using a newer version than 4.1, which is EOLed
wouterj
force-pushed
the
create_Fraemwork_fixes
branch
from
d8ae2f9 to
3f8e61d
Compare
|
PR updated and ready imo |
|
Thanks Wouter! The only thing I wonder about (and it was done this way before), is if we're going to use full namespaces for classes instead of use statements, why not prefix them with their opening \ so that they'll work even if someone copies some of this code into a namespaced file. What do you think about that? |
No description provided.