staabm / phpstan-todo-by

Todo comments with expiration
https://staabm.github.io/2023/12/17/phpstan-todo-by-published.html
MIT License
172 stars 4 forks source link

http caching #75

Closed staabm closed 7 months ago

staabm commented 7 months ago

we should have a http cache builtin, to reduce the impact of http requests on the phpstan analysis process. I think we just need to cache a url to ticket status mapping.

this will also make sure we won't hammer the APIs of the issue trackers from all todo-by users.

staabm commented 7 months ago

@EmilMassey @DannyvdSluijs any ideas/opinions? anyone of you guys willing to implement the caching?

EmilMassey commented 7 months ago

To be honest I'm very happy with the way it's cached currently by the phpstan itself. For me it's efficient enough and in my case API calls are made rarely thanks to the result cache.

How would this integrate with built-in phpstan cache? Would phpstan clear-result-cache invalidate tickets cache or it would be kept even after running this command?

Personally in my projects I'd probably set this parameter to disable cache, because I want to receive the feedback immediately. Hypothetically I might want to close/resolve the ticket and then run the phpstan (with result cache cleared) right away to make sure that all comments are resolved. If the status was cached at this moment, these comments wouldn't be reported and I would have false belief that there are no unresolved todos.

I don't know how it's going to work but I think cache would be nice feature to have if somebody wants/needs to have it enabled. These APIs when authenticated have quite big rate-limits, but indeed if the codebase has huge amount of todos and phpstan's isn't able to cache effectively, they could hit the limit. But I'd say this is quite rare scenario, so personally I'd prefer no cache as the default.

From the performance point of view, IMO #76 is more interesting but I don't know how to implement it.

staabm commented 7 months ago

From the performance point of view, IMO #76 is more interesting but I don't know how to implement it.

I agree, that this is the more interessting thing. I guess its pretty easy.. maybe I can implement it in the train on the weekend

github-actions[bot] commented 6 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.