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

Change ZendViewRenderer to DI only; remove fallback dependency setup #66

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented May 27, 2019

Provide a narrative description of what you are trying to accomplish:

This PR removes fallback PhpRenderer setup from ZendViewRenderer and moves NamespacedPathStackResolver setup into factory.
ZendViewRenderer requires zend-view renderer and NamespacedPathStackResolver to be explicitly provided at creation time; It won't try to inspect or configure zend-view renderer resolvers and expects them to be configured beforehand.

  • Are you creating a new feature?
    • Why is the new feature needed? What purpose does it serve?
      This greatly simplifies template renderer and forces separation of concern to avoid the need to modify template renderer to configure its dependencies as has been done in add support for custom template file default suffix #64
      Extra ZendViewRenderer parameter added in add support for custom template file default suffix #64 is dropped in this PR as that feature is moved into factory.
    • How will users use the new feature?
      Users depending on Zend\Expressive\ZendView\ConfigProvider to configure their container can continue to use the same provided services without change.
      Users directly instantiating Zend\Expressive\ZendView\ZendViewRenderer will need to update their code to provide zend-view renderer (PhpRenderer by default) and NamespacedPathStackResolver as constructor parameters; zend-view renderer is expected to be already configured with NamespacedPathStackResolver.
    • Base your feature on the develop branch, and submit against that branch.
    • Add only one feature per pull request; split multiple features over multiple pull requests
    • Add tests for the new feature.
    • Add documentation for the new feature.
    • Add a CHANGELOG.md entry for the new feature.

@Xerkus Xerkus added this to the 3.0.0 milestone May 27, 2019
@Xerkus Xerkus self-assigned this May 27, 2019
@weierophinney
Copy link
Member

This repository has been closed and moved to mezzio/mezzio-laminasviewrenderer; a new issue has been opened at mezzio/mezzio-laminasviewrenderer#1.

@weierophinney
Copy link
Member

This repository has been moved to mezzio/mezzio-laminasviewrenderer. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone mezzio/mezzio-laminasviewrenderer to another directory.
  • Copy the files from the second bullet point to the clone of mezzio/mezzio-laminasviewrenderer.
  • In your clone of mezzio/mezzio-laminasviewrenderer, commit the files, push to your fork, and open the new PR.
    We will be providing tooling via laminas/laminas-migration soon to help automate the process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants