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

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented May 28, 2019

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

This PR changes config option for resolver default suffix from default_suffix to extension to match plates and twig template renderers.
Related: #64

  • Are you creating a new feature?
    • Why is the new feature needed? What purpose does it serve?
      add support for custom template file default suffix #64 introduced a way to configure default suffix using configuration option $config['templates']['default_suffix'] however two other template renderers provided by framework use key extension
      This PR fixes inconsistency inadvertently introduced by add support for custom template file default suffix #64 by making use of extension key.
    • How will users use the new feature?
      By providing alternative suffix in config:
      [
          'templates' => [
              'extension' => 'php',
          ],
      ]
    • 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.
      Documentation needs to be added in another repository
    • Add a CHANGELOG.md entry for the new feature.

@Xerkus Xerkus added this to the 2.2.0 milestone May 28, 2019

- Nothing.
- [#67](https://github.com/zendframework/zend-expressive-zendviewrenderer/pull/67)
deprecates configuration option `$config['templates']['default_suffix']`
Copy link
Member

Choose a reason for hiding this comment

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

What about adding E_USER_DEPRECATED when default_suffix is used? It should help with migration.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be a bad idea to do for 'config' service. And I did a release just couple days ago. Missed that inconsistency during review.

@froschdesign
Copy link
Member

@Xerkus
I can not found any documentation for this configuration option for zend-view. Only for Plates and Twig:

It would be great if you could add this.

@Xerkus
Copy link
Member Author

Xerkus commented May 28, 2019

@froschdesign i didn't add it yet, I discovered inconsistency when I was preparing a PR for the docs

@froschdesign
Copy link
Member

@Xerkus

…I was preparing a PR for the docs

Great! 👍

@Xerkus Xerkus merged commit 9b3c532 into zendframework:develop Jun 4, 2019
Xerkus added a commit that referenced this pull request Jun 4, 2019
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