Skip to content

Conversation

@cezarpopa-cognita
Copy link
Contributor

Hi,

first i wanted to say thank you for this great plugin, as this is my first commit to the repository i hope it accepts pull requests.
I am using the plugin with WordPress VIP and due to the nature of the hosting setup i do not have write permissions on some folders.

Due to that, each time i visit a page the plugin will try to write to the read-only fs trigger dozens of notices for each request.

I have tested this and so far i can't see anything obvious that i have missed from the core functionality, having those checks in place so far seems to keep the functionality as is and keeps my logs a lot cleaner.

Thank you,
Cezar

@kprovance
Copy link
Member

kprovance commented Oct 11, 2023

Looks good. I don't see any issues and will give it a spin. That said, we still have about 30+ million users. It's the scenarios I can't predict that always get me. In the event there are widespread issues (because WP's filesystem API sucks), I'll have to roll it back and we can revisit the cause. I don't see that happening, but I've said that before and been bitten by it. ;-)

@kprovance kprovance merged commit 6c96590 into reduxframework:master Oct 11, 2023
@cezarpopa-cognita
Copy link
Contributor Author

Looks good. I don't see any issues and will give it a spin. That said, we still have about 30+ million users. It's the scenarios I can't predict that always get me. In the event there are widespread issues (because WP's filesystem API sucks), I'll have to roll it back and we can revisit the cause. I don't see that happening, but I've said that before and been bitten by it. ;-)

Thanks for that, i understand and wasn't aware the number of active users was that big.
On that note i am not sure if GitHub is just acting up. I have received a notification about a timeout in the class-redux-wp-filesystem.php which i can't see on the PR. Does the timeout still apply?

With regards to the scenarios, i will try to invest some time researching if we can get some form of tests in place maybe? Thanks again!

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