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

Final fixes for version 3 #29

Merged
merged 8 commits into from
Nov 30, 2017

Conversation

tux-rampage
Copy link
Contributor

This PR contains updates to the documentation and a small incompatibility fix:

  • Added a guide on how to use AoT in an Zend Expressive app
  • Fixed ServiceManager parameter incompatibility in generated AoT factories

I also created an example repository for the app in this guide: https://github.com/lukanetconsult/zend-di-aot-example

@@ -0,0 +1,112 @@
# Using AoT In A ZendFramework App
Copy link
Member

Choose a reason for hiding this comment

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

Whitespace is missing: "Zend Framework"

Copy link
Member

Choose a reason for hiding this comment

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

See also my question below:

Is this a "zend-expressive application" or "Zend Framework application"?


## Create project and add zend-di

For this guide will use an [expressive application](https://docs.zendframework.com/zend-expressive/)
Copy link
Member

Choose a reason for hiding this comment

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

Is this a "zend-expressive application" or "Zend Framework application"? I think there's a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought expressive was part of zend framework and an alternative to mvc. Anyway it's a good idea to change the title to make it more clear we're dealing with expressive here. I'll rephrase this paragraph accordingly.

Now add zend-di with composer:

```bash
composer require zendframework/zend-di=^3.0
Copy link
Member

Choose a reason for hiding this comment

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

Why the version constraint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it states clearly that this requires 3.0 at minimum and prevents composer to accidentally install zend-di 2.x.
Should there be more explanation about this?

Copy link
Member

Choose a reason for hiding this comment

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

If version 3.0 is released, then version 3 will be installed automatically. The version constraint is not needed. Remove it and all is fine.
We should not over complicated the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with both of you!

I'd suggest demonstrating without the version constant, but adding a note that if v2 gets installed, you should try the above (with constraint) to determine what is blocking it. You might also want to consider adding some verbiage in that note that if you are upgrading from v2, you will need to remove zend-servicemanager-di first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points.
I totally missed the zend-servicemanager-di conflict. I'll add a Note and hint to the version constraint option as sugessted by @weierophinney

Copy link
Member

Choose a reason for hiding this comment

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

For version hint: 👍

@@ -23,10 +23,11 @@ composer create-project zendframework/zend-expressive-skeleton zend-di-aot-examp
Pick the components you want to use. As statet above, we will be using Zend ServiceManager
and a Modular layout for this example.

Now add zend-di with composer:
Now add zend-di and zend-code with composer:
Copy link
Member

@froschdesign froschdesign Nov 30, 2017

Choose a reason for hiding this comment

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

At this point it is not clear why zend-code is needed. Please add an explanation in the sentence above. Thanks!

@tux-rampage
Copy link
Contributor Author

@froschdesign Thanks for your feedback. I'll check the Details later.

As stated by @froschdesign, this is dedicated to expressive, so state it
clearly in the title.
* Remove the version constraint from composer example
* Point out the conflict to zend-servicemanager-di
@weierophinney weierophinney merged commit 14f3e37 into zendframework:develop Nov 30, 2017
weierophinney added a commit that referenced this pull request Nov 30, 2017
weierophinney added a commit that referenced this pull request Nov 30, 2017
@weierophinney
Copy link
Member

Thanks, @tux-rampage! Edited for grammar and typography, but looks great!

@tux-rampage tux-rampage deleted the v3-finalization branch December 6, 2017 05:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants