Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Prepare skeleton to use ZF v3 releases by default #130

Merged
merged 54 commits into from Aug 15, 2016

Conversation

weierophinney
Copy link
Member

For the next minor release (1.4), we'll be using ZF v3 components by default. This patch does the followign:

  • Sets the minimum supported PHP version to 5.6.
  • Removes the zendframework/zendframework dependency entirely; zf-apigility and others bring in zend-mvc already, which gives us the MVC foundation necessary.
  • Adds zendframework/zend-component-installer as a requirement.
  • Adds zfcampus/zf-asset-manager as a development requirement.
  • Removes zendframework/zftool as a development requirement.
  • Updates all Apigility requirements, both production and development, to their latest stable versions.
  • Adds Composer scripts for:
    • enabling development mode (composer development-enable)
    • disabling development mode (composer development-disable)
    • reporting development mode status (composer development-status)

This has been tested quite a bit along with the zf-apigility-admin 1.5.0 release, but likely needs more testers. In particular, I'm curious what the "install" experience is like, and whether or not people are getting endless requests to update their module configuration, or if it "just installs".

- Use latest, known-stable versions that are forwards compatible with v3
  releases.
- Do not require all of zendframework.
- Proxy to zf-development-mode script installed via composer.
- Removes usage of `translate()` helper in shipped view scripts.
- Use `<?=` instead of `<?php echo` in shipped view scripts.
- Remove abstract factory entries for components not shipped by default.
- Add module entries for components that define them.
- Remove zf-tool dependency and module entry.
- Remove AssetManager dependency and module entry.
- Add zf-asset-manager as a dev dependency.
- Add zend-component-installer as a dependency.
- Adds .gitignore placeholder file in `public`.
- Picks up changes to zf-content-validation and zf-apigility-documentation.
- Updated to stable zf-apigility-admin, and removed zf-apigility-admin-ui (as
  it's a dependency of zf-apigility-admin).
@lrochas
Copy link

lrochas commented Aug 10, 2016

Not sure what kind of "install experience" feed back you're looking for but I can confirm that the basic installation worked like a charm:

  • Download the develop branch via zipball.
  • Unzip.
  • Install dependencies with Composer composer.phar install.
  • Put the skeleton in development mode php public/index.php development enable.
  • Start the built-in web server php -S 0.0.0.0:8080 -ddisplay_errors=0 -t public public/index.php.
  • Create new API and new Service then modify and delete.

Everything went smooth.

@@ -15,14 +15,14 @@
</head>
<body>
<div class="container">
<?php echo $this->content; ?>
<?= $this->content; ?>
Copy link
Member

Choose a reason for hiding this comment

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

usually we don't have ending ; in this type of expression

@benjaminkoetting
Copy link

same here

  • use the code as zip
  • composer install
  • composer development-enable
  • composer serve

it's working!

<hr/>
<h2><?php echo $this->translate('Additional information') ?>:</h2>
<h3><?php echo get_class($this->exception); ?></h3>
<h2>Additional information:</h2>
Copy link
Member

Choose a reason for hiding this comment

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

Why this part is not with 2 indents? It is body of if statement, so I think we should have:

<?php if (something) : ?>
    <some html="tag">...</some>
<?php endif ?>

Of course the same for loops.

Copy link
Member Author

Choose a reason for hiding this comment

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

Some of this indenting is inherited. Most changes I made were based on phpcs warning me about issues as I edited. I'll review again, however.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, this is different.

The markup should not be indented here, only the PHP statements. Otherwise, when viewing the source of the page, you'll have markup indented that shouldn't be.

In this particular case, we're conditionally displaying some markup, but that markup is not semantically inside any other blocks.

Copy link
Member

Choose a reason for hiding this comment

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

@weierophinney ok, it just looks odd in phtml files, but it make sense, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@webimpress Agreed, but it makes view source so much nicer when retrieving the final pages via HTTP. 😄

@weierophinney
Copy link
Member Author

weierophinney commented Aug 10, 2016

For those interested in testing composer create-project, you can do the following:

  1. Create a file named packages.json with the following contents:

    {
       "packages": {
           "zfcampus/zf-apigility-skeleton": {
               "1.4.0": {
                   "name": "zfcampus/zf-apigility-skeleton",
                   "version": "1.4.0",
                   "source": {
                       "url": "https://github.com/weierophinney/zf-apigility-skeleton.git",
                       "type": "git",
                       "reference": "9c166078bd5b51186e9e10b4d4c819639611190f"
                   }
               }
           }
       }
    }
  2. Run create-project, referencing the above as a repository:

    $ composer create-project --repository=packages.json zfcampus/zf-apigility-skeleton apigility 1.4.0

    This will create a new project in the directory apigility/.

  3. Descend into the project:

    $ cd apigility
  4. Put it into development mode:

    $ composer development-enable
  5. Test it:

    $ composer serve

    and visit http://localhost:8080/

I've done this locally, and found installation "just worked". Assets were correctly copied into the application, the installer did not prompt me to add modules, and the UI worked correctly.

Please let me know if you have any issues!

UPDATE

I've updated the sha1 reference to reflect recent commits pushed.

"zfcampus/zf-apigility-doctrine": "zfcampus/zf-apigility-doctrine ~1.0 to create Doctrine-Connected REST services",
"zfcampus/zf-http-cache": "zfcampus/zf-http-cache ~1.0 to add HTTP caching to your API",
"zfr/zfr-cors": "zfr/zfr-cors ~1.2 to add CORS support to your API"
"zfcampus/zf-apigility-doctrine": "zfcampus/zf-apigility-doctrine ^1.0 to create Doctrine-Connected REST services",
Copy link
Member

Choose a reason for hiding this comment

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

zfcampus/zf-apigility-doctrine is alredy in version 2, hopefully soon will be 2.1

@weierophinney
Copy link
Member Author

@lrochas

Put the skeleton in development mode php public/index.php development enable.

That worked? It shouldn't! With zf-development-mode v3, it should be one of:

  • composer development-enable
  • ./vendor/bin/zf-development-mode enable

I'm thinking you may have tested the develop branch, and not my branch specifically. Try the example I noted above, or:

$ git clone https://github.com/weierophinney/zf-apigility-skeleton.git
$ cd zf-apigility-skeleton
$ git checkout -b feature/zf-v3-readiness origin/feature/zf-v3-readiness
$ composer install
$ composer development-enable

- No tailing `;` if a closing `?>` is present.
- Ensure indentation is correct based on conditional/loop level.
- Usually, this will not be an issue, but this ensures no problems can
  occur. Per @webimpress.
- Suggested v2.1 instead.
@weierophinney
Copy link
Member Author

@webimpress — thanks for the detailed feedback; I've just pushed changes that should address each item you raised.

@weierophinney
Copy link
Member Author

For those of you testing via create-project, please note that I've updated the comment now to reflect the most recent sha1 pushed; update your packages.json to test against it! 😄

// Using __DIR__ to ensure cross-platform compatibility. Some platforms --
// e.g., IBM i -- have problems with globs that are not qualified.
'config_glob_paths' => array(realpath(__DIR__) . '/autoload/{,*.}{global,local}.php'),
'config_glob_paths' => [realpath(__DIR__) . '/autoload/{,*.}{global,local}.php'],
Copy link
Member

Choose a reason for hiding this comment

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

I think __DIR__ should be enough here, we don't need realpath here, am I wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

WRONG! 😄

IBM i Series users have reported to us in the past that globbing, even when using zend-stdlib, fails unless the real path is used. As such, we provide it both in here and the ZF skeleton application.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I didn't it, thanks ! :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I didn't either until I got the issues in the tracker, and one of our pro services folks did the research. It seems an edge case, but we have a significant number of IBM i users.

- Use ubuntu/xenial64 as the base.
- Inline the additions to the default vhost, instead of requiring another file.
- Document vagrant + virtualbox requirements on linux to use the image.
- Ensure UI module is up-to-date.
- Inline the default vhost
- Add `ubuntu-xenial` to the `/etc/hosts` file to fix a bug
- Use the `-q` switch with grep to prevent the need for redirecting output in
  conditionals
- Conditionally install composer; if it's already present, do a self-update
@weierophinney
Copy link
Member Author

Besides the instructions noted above for using create-project, you can also, once the project is created, use either docker-compose or vagrant:

# docker-compose:
$ docker-compose build # only needed the first time
$ docker-compose up
# vagrant:
$ vagrant up

Please note that the vagrant image is ubuntu/xenial64; if you are using VirtualBox, you will need vagrant 1.8.5 or higher, and VirtualBox 5.0.26 or higher.

Simplifies documentation, and you can enable/disable development mode easily via
the container.
- Uses PHP_EOL now.
- Also updates the docker-compose directions
Adds a delegator factory for the `ZF\Configuration\ConfigWriter` service
that configures it to use short array notation and `::class` constants.
The delegator is only enabled in development mode.
'config_cache_enabled' => false,
'config_glob_paths' => [
__DIR__ . '/../../../../config/autoload/{,*.}{global,local}.php',
__DIR__ . '/../../../../config/autoload/{,*.}{global,local}-development.php',
Copy link
Member

Choose a reason for hiding this comment

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

globbing, IBM etc... ? realpath(__DIR__) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessary in the unit tests.

- zf-apigility-admin-ui 1.3.6
- zf-apigility-admin 1.5.4
- zf-asset-manager 1.1.1
- zf-composer-autoloading 1.0
- eliminates the need for the delegator factory!
- zfcampus/zf-apigility-admin 1.5.6
- zfcampus/zf-apigility-admin-ui 1.3.7
- No longer necessary to include it as of 1.2.0.
*/

return array(
'view_manager' => array(
use Apigility\ConfigWriterDelegatorFactory;
Copy link
Member

Choose a reason for hiding this comment

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

it can be removed now

@viyirenov
Copy link

viyirenov commented Aug 15, 2016

I tried your instructions and it ran nicely, two things though:

1 - composer serve script has to be with public and not public/ to work on Windows otherwise it will throw an error saying that the directory doesn't exist.

"scripts": {
    // ...
    "serve": "php -S 0.0.0.0:8080 -t public public/index.php",
    // ...
}

2 - Error when trying to require doctrine orm module:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Can only install one of: zendframework/zend-hydrator[2.2.1, 1.1.0].
    - Can only install one of: zendframework/zend-hydrator[1.1.0, 2.2.1].
    - Can only install one of: zendframework/zend-hydrator[1.1.0, 2.2.1].
    - doctrine/doctrine-orm-module 1.0.0 requires zendframework/zend-hydrator ^1.1 -> satisfiable by zendframework/zend-hydrator[1.1.0].
    - Installation request for doctrine/doctrine-orm-module ^1.0 -> satisfiable by doctrine/doctrine-orm-module[1.0.0].
    - Installation request for zendframework/zend-hydrator (locked at 2.2.1) -> satisfiable by zendframework/zend-hydrator[2.2.1].

Installation failed, reverting ./composer.json to its original content.

- Remove tooling-specific entries (those should be in global configuration, or
  can be added after starting a project)
- Moved `config/autoload/` rules to that directory
- Removed unused rules
- It's only required in development anyways.
@weierophinney weierophinney merged commit 721ea7b into zfcampus:develop Aug 15, 2016
weierophinney added a commit that referenced this pull request Aug 15, 2016
@weierophinney weierophinney deleted the feature/zf-v3-readiness branch August 15, 2016 12:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants