[garbage-collector] Initial commit.#649
Conversation
…s also covers add-on–related use cases; properly store updated options and store them only once).
swashata
left a comment
There was a problem hiding this comment.
@fajardoleo Thanks for the PR. I think I understand the logic, but I do believe the code can be made more organized. Please see my suggestion, thanks 👍
…ating large method(s) into multiple smaller methods.
swashata
left a comment
There was a problem hiding this comment.
@fajardoleo Great job on the refactoring 👏 . Please see my few suggestions, I think we can simplify it/make it more readable. Thanks
swashata
left a comment
There was a problem hiding this comment.
@fajardoleo Nice refactoring. You have managed to get rid of several if..else in the logic and the code is much more readable now. Please see my suggestion for some easy improvements. Thanks.
| } | ||
|
|
||
| if ( ! $updated ) { | ||
| if ( isset( $option[ $slug ] ) ) { |
There was a problem hiding this comment.
Just a question @fajardoleo : Are all these checks mutually exclusive? So when $option[ $slug ] is present we won't need to check for $option[ "{$slug}:{$this->get_type()}" ]?
There was a problem hiding this comment.
Thanks @fajardoleo looks like I get it now. If we ever go with my improvement suggestion (one GC class per option) we can get rid of this, but not important right now at the moment. I have added a @todo.
5be4d83 to
b2c48da
Compare
…age collect.
1. Consider {product}_data when doing garbage collection.
2. Don't garbage collect all_{module} and active_{module} since those
have different structure and source. Add @todo to implement them
later.
3. Properly handle missing timestamp in the garbage collector, adding
zero risk to the existing BL.
4. Disable garbage collection by default and add it behind a flag =>
WP_FS__ENABLE_GARBAGE_COLLECTOR.
5. Simplify some logic where possible.
swashata
left a comment
There was a problem hiding this comment.
@fajardoleo I implemented my suggestions and fixed the typos I found. Now please review my commit and test :D
| } | ||
|
|
||
| if ( ! $updated ) { | ||
| if ( isset( $option[ $slug ] ) ) { |
There was a problem hiding this comment.
Thanks @fajardoleo looks like I get it now. If we ever go with my improvement suggestion (one GC class per option) we can get rid of this, but not important right now at the moment. I have added a @todo.
…erridden during operation and how the gc timestamp is stored.
No description provided.