Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP][Cache Resolver] Implement relative web path resolver #1233

Open
wants to merge 17 commits into
base: 2.x
Choose a base branch
from

Conversation

franmomu
Copy link
Contributor

@franmomu franmomu commented Oct 5, 2019

Q A
Branch? 2.0
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

This is #1059 merged with the current master, I fixed the tests and conflicts, but I haven't checked the implementation yet.

EDIT:

  • The constructor of WebPathResolver shouldn't be change in case someone has extended this class.
  • I would just instantiate the PathResolver inside the class and I'm not sure if the PathResolverInterface is needed.

@franmomu franmomu changed the title [Cache Resolver] Implement relative web path resolver [WIP][Cache Resolver] Implement relative web path resolver Oct 10, 2019
@icedevelopment
Copy link

Hey @nmuntyanov @franmomu @michellesanver
Can anybody tell me what's the status of this PR? Is anyone still working on it? What's preventing it from being merged?
This would be an interesting feature to have.

@franmomu
Copy link
Contributor Author

Hi, I don't think someone is working on it, feel free to take over. Based on the comments I wrote:

The constructor of WebPathResolver shouldn't be change in case someone has extended this class.

This should be done in a BC-break way.

I would just instantiate the PathResolver inside the class and I'm not sure if the PathResolverInterface is needed.

Check if it's really needed the interface.

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

Successfully merging this pull request may close these issues.

3 participants