-
Notifications
You must be signed in to change notification settings - Fork 100
patch: caching bug fixed #180
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
Conversation
Signed-off-by: Vedansh Saini <[email protected]>
Reviewer's GuideReplaced bitwise operators with logical AND in the allIncluded function’s cache validation and token logic to ensure the cache is correctly invalidated when token state changes. Class diagram for updated cache validation logic in scrumHelper.jsclassDiagram
class scrumHelper {
+allIncluded(outputTarget)
}
class githubCache {
+data
+cacheKey
+usedToken
}
class githubToken
scrumHelper --> githubCache : uses
scrumHelper --> githubToken : checks
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @vedansh-5 - I've reviewed your changes - here's some feedback:
- Consider adding an ESLint rule or custom lint check to flag accidental use of bitwise
&in boolean expressions to prevent similar bugs. - Add a focused unit test for the branch that invalidates the cache when a token becomes available after the cache was fetched without one.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding an ESLint rule or custom lint check to flag accidental use of bitwise `&` in boolean expressions to prevent similar bugs.
- Add a focused unit test for the branch that invalidates the cache when a token becomes available after the cache was fetched without one.
## Individual Comments
### Comment 1
<location> `src/scripts/scrumHelper.js:359` </location>
<code_context>
const needsToken = !!githubToken;
const cacheUsedToken = !!githubCache.usedToken;
- if (githubCache.data && isCacheFresh & isCacheKeyMatch) { //should be && check after rebase
- if (needsToken & !cacheUsedToken) {
+ if (githubCache.data && isCacheFresh && isCacheKeyMatch) {
+ if (needsToken && !cacheUsedToken) {
</code_context>
<issue_to_address>
Bitwise AND replaced with logical AND for boolean check.
Using '&&' instead of '&' ensures proper boolean evaluation and prevents unintended behavior when operands are not strictly boolean.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
|
Works properly now, thanks @Preeti9764 for pointing it out, since I had no open PRs I couldn't check it, and I try it with different timeline assuming it would be working as expected given that I had not touched the code for the concerned feature. |
|
Igtm ! Thanks. |
|
@hpdang Resolved the conflicts. |


📌 Fixes
Fixes #179
Caching works as intended now.
📸 Screenshots / Demo (if UI-related)
cacheFix.mp4
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Bug Fixes:
&with logical&&in cache conditionals to correctly evaluate cache freshness and key matching.