-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Prepare for zend-mvc v3 #329
Prepare for zend-mvc v3 #329
Conversation
- Removed configuration for non-default zend-mvc features, including translation, forms, etc. - Removed translation files. - Removed translation within templates. - Re-organized Application module as a PSR-4 directory, and added an autoloading rule for it.
- Adds a `VERSION` constant to the `Application\Module` class; this is now consumed by the home page template. - Updates dependencies to pick up zend-view 2.6.7 and zend-router 3.0.1. - Updates the base application config to include the router and validator components. - Simplifies the autoloader to *only* use composer.
This is a WIP; do not merge until zend-mvc is tagged and/or zendframework is tagged! |
|
||
if (!class_exists('Zend\Loader\AutoloaderFactory')) { | ||
throw new RuntimeException('Unable to load ZF2. Run `php composer.phar install` or define a ZF2_PATH environment variable.'); | ||
if (! class_exists('Zend\Mvc\Application')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
::class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not move that code in index.php? Imo it will simplify things a little more.
A few notes here:
|
A few other notes:
|
Very nice, it looks very clear and I like the way other zend-components are registered as modules. Removing one directory level is also very nice. We developers are lazy and like every small thing which makes our lives easier. For the skeleton, I would convert all array(...) to use short array syntax. Old arrays exists on config files and in the layout.phtml. Thanks for good work guys! |
</div> | ||
<div class="panel-body"> | ||
<p><?php echo sprintf($this->translate('If you need any help or support while developing with ZF2, you may reach us via IRC: %s#zftalk on Freenode%s. We\'d love to hear any questions or feedback you may have regarding the beta releases. Alternatively, you may subscribe and post questions to the %smailing lists%s.'), '<a href="irc://irc.freenode.net/zftalk">', '</a>', '<a href="http://framework.zend.com/wiki/display/ZFDEV/Mailing+Lists">', '</a>') ?></p> | ||
<p><a class="btn btn-success pull-right" href="http://webchat.freenode.net?channels=zftalk" target="_blank"><?php echo $this->translate('Ping us on IRC') ?> »</a></p> | ||
<p><?php echo sprintf('If you need any help or support while developing with ZF2, you may reach us via IRC: %s#zftalk on Freenode%s. We\'d love to hear any questions or feedback you may have regarding the beta releases. Alternatively, you may subscribe and post questions to the %smailing lists%s.', '<a href="irc://irc.freenode.net/zftalk">', '</a>', '<a href="http://framework.zend.com/wiki/display/ZFDEV/Mailing+Lists">', '</a>') ?></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/beta release/this release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/the beta releases/this release
)); | ||
} | ||
} | ||
include 'vendor/autoload.php'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as no file_exists check, I think @
prefix ( @include
usage ) should be provided as the error needs to be shown to user is the RuntimeException error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHP will barf all over the place at this point anyways, and you'll get both an E_WARNING
and the following RuntimeException
. Essentially, I don't want to suppress errors or do additional error checking for something that represents a fundamentally broken install anyways.
@weierophinney I created PR weierophinney#3 to your branch for some feedbacks above ( wording, ::class, short array syntax ) |
|
||
class Module | ||
{ | ||
const VERSION = '3.0.0dev'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is shameless advertising, but ocramius/package-versions
deals with it, eventually :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's slick!
/me goes to add that in the next push...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DAMMIT, @Ocramius ... PHP 7 as the minimum requirement?!?!?! (now I understand the "eventually" comment, though...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, totally forgot about that, sorry :-)
Yeah, next time then :-P
@Ocramius — some responses:
The
One consistent complaint I've heard is from developers who start with the skeleton, and then need to strip out a whole bunch of stuff they don't need. If we have a minimal base on which we can expand, we solve that problem.
It's used by the
Thoughts?
Interestingly, the |
@Ocramius — more responses:
It's currently required by zend-modulemanager, for autoloading modules that do not have autoloading setup. (See Since one goal is to have a clear and clean upgrade path, we currently need to keep that. What we could do, however, is deprecate the usage; the question is if we should do that in a 2.x series of zend-modulemanager, or in the v3 release. If we do it in the 2.x series, we could emit a deprecation notice from Thoughts?
Out of the box, no, not for the minimal skeleton. zend-psr7bridge is listed as a suggested requirement for zend-mvc already, however, and the suggestion text indicates it should be installed if you plan to use PSR-7 middleware within your application.
Yes and no. zend-component-installer prompts you to add the component, but gives you the option of saying "no" (as well as saying, "remember my selection for all remaining components on this installation run). It also checks to see if the component is already listed, and, if so, will not prompt. My feeling is that components required by the base skeleton should be listed in the configuration as well, so that you only get prompted the first time you add a new component. |
Noted; can't believe I missed that on the first pass through! |
ZendSkeletonApplication#329 feedbacks
Agreed, let's keep i18n out then.
It should probably live in the
Fair enough. I feel like we should push further on this in future, but maybe not now.
My thought here is that while the user may answer "no", he may actually learn that he will be prompted. Not having that is also fine, IMO, but we're committing post-install artifacts. Still: good for usability, let's keep it as-is, I'd say. |
Would it be that crazy if the "installer" and skeleton was merged with zend-expressive-skeleton ? If it was, there are possible cons:
Pros:
Current blog posts and available docs do not clearly explain the PSR7 transition IMO. This would effectively solve the problem as ZF3 skeleton would give one clear choice. Through the "power of defaults" (default = expressive), the installer would be educating and promoting PSR7. |
I think for 'controllers' -> 'invokables' config, we can use factories and use |
@@ -0,0 +1,16 @@ | |||
FROM php:7.0-apache | |||
|
|||
RUN apt-get update \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this Dockerfile intended for use in production?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most likely you would end up modifying it for production to include your application specific needs. The primary intent is to allow developers to start with a consistent development environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could split the command for make it more simple and add more things to docker cache even if it add more layers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is doing exactly three things:
- Ensuring we have the extensions necessary to run composer.
- Ensuring the web server is configured correctly (enables mod_rewrite and changes the document root).
- Installing composer.
I'm not sure we really need to simplify it further…
- Port 80 is already exposed by the parent container - docker-composer defines the volume
- Per @Maks3w, added a `cd /var/www` command into the vagrant user's profile; simplifies running commands.
- Details various ways to install dependencies.
This is ok for me 👍 |
- zend-mvc is now at 3.0.0 - zend-test is now at 3.0.0 - zend-mvc-console is now at 1.1.10 - zend-mvc-i18n is now at 1.0.0 - zend-mvc-plugin-* are now at 1.0.0
- Instead of individual packages for each plugin.
Status updateFirst, I've updated to stable versions of:
This puts the pull request in a good position to be tagged once merged. However… I've created a zend-mvc-plugins metapackage, to aggregate all official plugins, reducing the number of prompts by three. However, on experimentation, it appears that packages installed via a metapackage do not trigger plugins; instead, only the metapackage triggers the plugin. The solution will be to allow the |
- skeleton installer: Prefer ^1.0 or ^0.1.2 - component installer: Prefer ^1.0 or ^0.2 - organize optional packages by package name
- first package to define component installer metadata
Status update
At this time, this patch is finally ready to merge! |
Pushed to develop for release with 3.0.0. |
For the v3 release, we will be pulling in a smaller set of components out-of-the-box. These are primarily defined by zend-mvc, but also include:
zend-config (used by the zend-modulemanager(zend-modulemanager now defines the zend-config requirement)ConfigListener
by default)Additionally, we plan to leverage Composer for all autoloading, which means:
Application
module, and remove one directory level;Application
module no longer needs to setup autoloading.For a minimal application, we do not want or need internationalization. As such, this patch also removes all translations and calls to translate strings in templates. We can explore re-adding them later, or offering an alternative skeleton application with more default features (including i18n, forms, databases, etc.).
At the time of submission, this patch provides a working skeleton with the current zend-mvc develop branch.
Testing
If you wish to test:
git checkout -b develop origin/develop
wget https://patch-diff.githubusercontent.com/raw/zendframework/ZendSkeletonApplication/pull/329.patch && git am 329.patch
(or, if you use the hub command:git merge https://github.com/zendframework/ZendSkeletonApplication/pull/329
)composer install
If you want to use vagrant:
vagrant up
vagrant ssh -c 'cd /var/www ; composer install'
If you want to use docker:
docker-compose up -d --build
docker-compose run zf composer install
Please report any issues you notice or provide constructive feedback via comments to this pull request, or issue a pull request against my branch.
TODO
Installer
In each case, this should register the module(s) with the application, so that the user does not need to be prompted twice.
Documentation
/[:controller][/:action]]
route was removed from the skeleton. Again, it will not affect existing users, but will affect experienced users who originally relied on it being active in new skeleton projects.These items have been added to the maintainers wiki.
Updates
Close #293