-
Notifications
You must be signed in to change notification settings - Fork 82
Scrum report is now being generated when the extension is freshly unpacked. #153
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Reviewer's GuideInstead of relying solely on existing storage data, the PR enhances initialization by reading form inputs on unpack and merging them into local storage, refactors storage/cache handling using dedicated helpers, enriches GitHub API fetch logic with token support and queuing, and defers report injection until all data is ready to ensure reports generate correctly upon fresh unpacking. File-Level Changes
Assessment against linked issues
Possibly 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 and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
I have considered Sourcerys' suggestions. |
Signed-off-by: Vedansh Saini <[email protected]>
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.
@vedansh-5 @hpdang for me the current changes did not solve the issue, it still needs refresh after loading and works fine.Thanks!
Signed-off-by: Vedansh Saini <[email protected]>
@Preeti9764 Please check the latest changes to see whether they fix the issue for you or not. Thanks! |
@vedansh-5 No proper handling in case of username not provided, some unknown elements appears in ui .Can you fix that.Thanks! |
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.
there is as scope of race condition in cache loading if multiple calls happened simultaneously. The loadFromStorage() call is not protected by the fetching flag.
Signed-off-by: Vedansh Saini <[email protected]>
@Preeti9764 This is not a practical problem as the mentioned |
I have fixed this, can you take a look @Preeti9764 , Thanks! |
Fixes #148
📝 Summary of Changes
Earlier the extension was using locally saved data to generate a scrum report. This logic was resulting in the scrum report not being generated when the extension was freshly unpacked into the chrome extension module.
In this PR I have implemented to use the data present in form section to fetch the report which is then saved into the local browser storage, with this approach, the extension properly generates the scrum report when it is freshly unpacked.
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Use form inputs to seed local storage when empty, ensuring scrum reports generate on first load; refactor storage and GitHub data caching logic for robust cache management and token support.
Bug Fixes:
Enhancements: