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

LinkGenerator: extract interface #273

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

PavelJurasek
Copy link
Contributor

Since LinkGenerator is final, it would be nice if it had its own interface so that it can be replaced with own implementation.

@dg
Copy link
Member

dg commented Jan 15, 2021

What is the interface for? Replace it with your own implementation.

@PavelJurasek
Copy link
Contributor Author

Right, I can change service application.linkGenerator to whatever class I want, but it won't solve the problem with 3rd party packages that are typehinted to Nette\Application\LinkGenerator. That leads me to conclusion, that the PR should be reworked to:

  • create ILinkGenerator interface
  • create alias LinkGenerator for ILinkGenerator to keep BC
  • rename current class implementation, DefaultLinkGenerator or similar

@dg
Copy link
Member

dg commented Jan 19, 2021

@PavelJurasek I'll save this as a beautiful argument when someone wonders why I'm removing the letter I from the interface names. :-)

@dg dg force-pushed the master branch 6 times, most recently from 13f5ab3 to c15bd94 Compare January 26, 2021 21:24
@dg dg force-pushed the master branch 9 times, most recently from 7de5809 to f817a0b Compare February 8, 2021 05:57
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.

2 participants