-
Notifications
You must be signed in to change notification settings - Fork 78
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
fixes: Added feature to show commits #55
base: master
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new feature to fetch and display commit information from GitHub for a given contributor. The implementation involves constructing the proper API URL, performing an AJAX call to retrieve commits under a specified time period, and then processing and displaying the commit details in the scrum board. Sequence diagram for fetching and displaying GitHub commitssequenceDiagram
participant Browser
participant ScrumHelper
participant GitHubAPI
%% User triggers the scrum board refresh
Browser->>ScrumHelper: Trigger allIncluded()
ScrumHelper->>ScrumHelper: Initialize variables (githubUsername, projectName, dates)
ScrumHelper->>ScrumHelper: Construct commitsUrl
ScrumHelper->>GitHubAPI: $.ajax GET(commitsUrl)
GitHubAPI-->>ScrumHelper: Return commit data
ScrumHelper->>ScrumHelper: Store commit data in githubPrCommitsData
ScrumHelper->>ScrumHelper: Call writeGithubCommits()
Note right of ScrumHelper: Loop through commits, format HTML list items
ScrumHelper->>Browser: Update scrum board with commit information
File-Level Changes
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 @Harshdev098 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider replacing the polling via setInterval with a promise-based or async/await approach to handle API call completion.
- Add error handling logic in your AJAX call for fetching commits instead of leaving the error callback empty.
- Validate that githubPrCommitsData is an array before iterating to avoid potential runtime errors.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@mariobehling I have commited the changes to fetch the commits for fixing issue #51. Can you please review it. |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@hongquan Done with the changes! |
Please run Biome to format your code. |
@hongquan Done ✅ |
Is this issue still open? |
@hongquan Done with the changes |
Could you look back and check your work, there are many occurence of "inconsistent style of variable name". |
Done with the required changes @hongquan |
Fixes issue #51
Changes:
Added a function to fetch commits of the contributor with the repository name under the specified time period
Summary by Sourcery
New Features: