Skip to content

[Cache Resolver] Implement relative web path resolver - #1059

Closed
nmuntyanov wants to merge 16 commits into
liip:masterfrom
nmuntyanov:2.0
Closed

[Cache Resolver] Implement relative web path resolver#1059
nmuntyanov wants to merge 16 commits into
liip:masterfrom
nmuntyanov:2.0

Conversation

@nmuntyanov

@nmuntyanov nmuntyanov commented Feb 20, 2018

Copy link
Copy Markdown

#| Q | A
| --- | ---
| Branch? | 2.0
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #1050
| License | MIT

Added new path resolver for relative paths creation
(RelativeWebPathResolver && RelativeWebPathResolverFactory)

usage example:

liip_imagine:
  resolvers:
     default:
       relative_web_path:
         web_root:  "%kernel.root_dir%/../web"
         cache_prefix: assets

@makasim

makasim commented Feb 20, 2018

Copy link
Copy Markdown
Contributor

It would better to register a new resolver for that, instead of adding a bool flag.

@nmuntyanov nmuntyanov changed the title 2.0 RelativeWebPathResolverAdded Feb 20, 2018
@robfrawley robfrawley changed the title RelativeWebPathResolverAdded [Cache Resolver] Implement relative web path resolver Feb 21, 2018
@robfrawley robfrawley added Level: New Feature 🆕 This item involves the introduction of new functionality. Type: Documentation This item pertains to documentation of this project. State: Reviewing This item is being reviewed to determine if it should be accepted. Type: Source Code This item pertains to the source code of this project. labels Feb 21, 2018
@robfrawley robfrawley added this to the 2.0.0 milestone Feb 21, 2018
@robfrawley

robfrawley commented Feb 21, 2018

Copy link
Copy Markdown
Collaborator

Once we work out all the code implementation details (which I'll provide my comments on shortly, once I get a chance to review everything), be advised that we only accept PRs that includes documentation.

You should add the appropriate files in the Resources/doc directory and you can build the RST files to HTML by following the Wiki article for local testing. The documentation is important and is auto-generated from the RST files for https://symfony.com/doc/master/bundles/LiipImagineBundle/index.html.

@maximgubar

Copy link
Copy Markdown
Contributor

@nmuntyanov please resolve conflicts on this PR, so it could be processed further.

nmuntyanov added 2 commits May 1, 2018 12:43
# Conflicts:
#	Tests/DependencyInjection/ConfigurationTest.php
#	Tests/DependencyInjection/Factory/Resolver/WebPathResolverFactoryTest.php
#	Tests/DependencyInjection/LiipImagineExtensionTest.php
#	Tests/Imagine/Cache/Resolver/WebPathResolverTest.php
@nmuntyanov

Copy link
Copy Markdown
Author

Resolved

@nmuntyanov

Copy link
Copy Markdown
Author

Tests were fixed here: #1087

@nmuntyanov

Copy link
Copy Markdown
Author

@robfrawley check this PR please.

@maximgubar

Copy link
Copy Markdown
Contributor

since default branch was changed to master, please update the base branch for this PR

@nmuntyanov
nmuntyanov changed the base branch from 2.0 to master June 26, 2018 15:39
@nmuntyanov

Copy link
Copy Markdown
Author

@maximgubar updated

@icedevelopment

Copy link
Copy Markdown

Any news regarding this PR? @maximgubar @nmuntyanov
Would be really nice to have this feature.

@s-titov

s-titov commented Sep 17, 2019

Copy link
Copy Markdown

Any news? @nmuntyanov

@michellesanver

Copy link
Copy Markdown
Contributor

Replaced by #1233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Level: New Feature 🆕 This item involves the introduction of new functionality. State: Reviewing This item is being reviewed to determine if it should be accepted. Type: Documentation This item pertains to documentation of this project. Type: Source Code This item pertains to the source code of this project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants