-
Notifications
You must be signed in to change notification settings - Fork 90
PATCH : changed title in issues and PRs #228
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]>
Reviewer's GuideThis PR refactors the date-based logic in scrumHelper.js to correctly distinguish items created today from those updated within the reporting period. It introduces a normalized “isCreatedToday” flag and updates the PR and issue action labels accordingly, replacing stale ‘Existing/Made/Open’ terms with dynamic ‘Updated’ or ‘Made/Open’ based on creation date. Class diagram for scrumHelper.js issue and PR label logic updateclassDiagram
class ScrumHelper {
+generateScrumReport()
+isCreatedToday(item: Object): Boolean
}
class Item {
+created_at: Date
+updated_at: Date
+state: String
+state_reason: String
}
ScrumHelper --|> Item : uses
ScrumHelper : +"if isCreatedToday(item) => 'Opened Issue'/'Made PR'"
ScrumHelper : +"else => 'Updated Issue'/'Updated PR'"
File-Level Changes
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 - here's some feedback:
- The new
isNewPR
check (prCreatedDate == new Date()
) will always be false – switch back to a proper date-range filter or normalize and compare only the date parts. - You have the same logic for zeroing out hours and checking
isCreatedToday
in two places – extract that into a helper function to avoid duplication. - Right now closed issues still use
issueActionText
based on creation date; it might be clearer to use a distinct "Closed Issue" label instead of reusing the opened/updated logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `isNewPR` check (`prCreatedDate == new Date()`) will always be false – switch back to a proper date-range filter or normalize and compare only the date parts.
- You have the same logic for zeroing out hours and checking `isCreatedToday` in two places – extract that into a helper function to avoid duplication.
- Right now closed issues still use `issueActionText` based on creation date; it might be clearer to use a distinct "Closed Issue" label instead of reusing the opened/updated logic.
## Individual Comments
### Comment 1
<location> `src/scripts/scrumHelper.js:1427` </location>
<code_context>
+ const isCreatedToday = today.getTime() === itemCreatedDate.getTime();
- const isNewPR = prCreatedDate >= startDateFilter && prCreatedDate <= endDateFilter;
+ const isNewPR = prCreatedDate == new Date();
const prUpdatedDate = new Date(item.updated_at);
const isUpdatedInRange = prUpdatedDate >= startDateFilter && prUpdatedDate <= endDateFilter;
</code_context>
<issue_to_address>
Direct comparison of Date objects may not work as intended.
Use getTime() to compare the numeric values of the dates, or normalize both dates before comparison to ensure accurate value equality.
</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]>
@vedansh-5 I tested this but there was a mismatch between my Compose Window and the Scrum Generation, please see screenshot |
@Preeti9764 can you plz review this? |
Signed-off-by: Vedansh Saini <[email protected]>
@Preeti9764 @hpdang Please review, thankyou |
@Preeti9764 could you please review this? |
📌 Fixes
Fixes #227
Fixes #217
As we discussed in our weekly scrum meeting to have the title of issues and PRs changed to updated instead of made/ opened. This PR changes the code responsible for the same.
📝 Summary of Changes
Here I have changed the logic to display Opened Issue and Made PR when the respective are created today, if we generate the scrum report for a week in which the issues and PRs were made they would still display updated in their title as they were created on some other date.
📸 Screenshots / Demo (if UI-related)
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Revise the action labels for issues and pull/merge requests to differentiate between items created today and those updated in the report period.
Enhancements: