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

Make Pagefile::setFilename hookable #954

Closed
BernhardBaumrock opened this issue Aug 6, 2019 · 3 comments
Closed

Make Pagefile::setFilename hookable #954

BernhardBaumrock opened this issue Aug 6, 2019 · 3 comments

Comments

@BernhardBaumrock
Copy link

Please see https://processwire.com/talk/topic/22068-pw-30137-%E2%80%93%C2%A0core-updates/?do=findComment&comment=189468

@BernhardBaumrock
Copy link
Author

@ryan any news on this? It works really great in my setup and I think this approach can open up a lot of great options for a better DEV/LIVE workflow with processwire. In my current project where I modified the core file it was really just doing a

git clone myrepo.git
git submodule update --init --recursive
# restore db dump

And I had a working copy of the site that downloaded all assets on demand while working with it! This is really awesome but it would break on every core update. I could implement some hack that does a str_replace and file_put_contents in my dev environment to prevent updates breaking this feature, but it's really just adding three underscores to one core file thanks to the ingenious hook system 🙂

https://processwire.com/talk/topic/22068-pw-30137-%E2%80%93%C2%A0core-updates/?do=findComment&comment=190216

@ryancramerdesign
Copy link
Member

@BernhardBaumrock I'm working through issue reports this week, came across this request and just wanted to check if you still needed this hookable? The thread you linked above seemed to indicate you might not need it anymore, so wanted to check. There's a little bit of overhead with every hookable method (relative to a regular method), so for a method that gets called a lot like this one, if there's already another way to accomplish what's needed without the hook, it's better if we don't make it hookable. On the other hand, if the only way to solve a recurring need is by making the method hookable, then we should do so.

@BernhardBaumrock
Copy link
Author

Thx, it was my fault not having the newest PW version therefore it didn't work as expected!

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

No branches or pull requests

2 participants