Made GitHub cache update async and fixed cache refresh check. Fixes #… by waliamehak · Pull Request #1316 · Together-Java/TJ-Bot
Fixes:
- Made updateCache() async to avoid blocking.
- Added null-safe default for autocompleteGHIssueCache as asked.
Also did:
- Added null-safe default for lastCacheUpdate (Instant.EPOCH) to prevent NPE.
- Fixed cache refresh logic by using isBefore (only refresh when stale).
Hey @Zabuzard, could you please review this PR?
Hey @Alathreon, I guess #1317 means this fix is not needed anymore? so, should i close this? Pretty neat solution btw.
Hey @Alathreon, I guess #1317 means this fix is not needed anymore? so, should i close this? Pretty neat solution btw.
We want both. The code being blocking is not "correct". The page size being suboptimal isnt good either. Both have to be tackled :)
Hi @Zabuzard, updated the commit accordingly. Let me know if everything is up to speed now :)
@waliamehak waliamehak force-pushed the develop branch from 15db2a7 to 11671ea 18 hours ago
FYI after a code review has started, force-push should not be done anymore. The problem is that a force-push changes commit history. So GitHub cannot tell me now anymore which commits I already reviewed and which contain actual new changes.
Like, If I click on "rewiew commits since my last review" I now get this:
So I have to essentially restart my review from scratch now, instead of doing it incremental. Fortunately, this PR is relatively small so its not that big of a deal. But you gotta keep that in mind 👍
Better not do any force-push after you received a CR anymore and just do regular merges at that point 🙂
@waliamehak Looks like the file has a minor spotless/style issue. Running gradle spotlessApply would probably autofix it 🙂
(we can also do it for you if you prefer that)
Thanks, @Zabuzard, I'll avoid force pushes in the future. Also, Spotless fails on some pre-existing _ identifiers, these are not changes from this PR so ignoring these so we can proceed with review; all other formatting issues are clean.
Thanks, @Zabuzard, I'll avoid force pushes in the future.
Only after a CR. Before a CR they are completely fine 👍
Also, Spotless fails on some pre-existing _ identifiers, these are not changes from this PR so ignoring these so we can proceed with review;
Not on my end, mh... Sounds like you are potentially on the wrong Java version somewhere, as this was a very recent addition.
all other formatting issues are clean.
Great, thanks for your efforts! 🙂
@Zabuzard, you are right. Switching to JDK 24 resolves them :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
