-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Maintenance and adjustment to "Schedule Monthly" workflow #7638
Maintenance and adjustment to "Schedule Monthly" workflow #7638
Conversation
Adjustments made to `oneMonthAgo` for Feb and Aug runs, also add "Skills Issue"
Change "Pre-work checklist" --> "Skills Issue"
Change "Pre-work Checklist" --> "Skills Issue", add REST API ref comments, remove excess calc for `currentTeamMembers`
Changing comment "Pre-work Checklist" --> "Skills Issue"
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
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 @t-will-gillis this looks great!
- The branch name is correctly
- The issue is linked as well
- The console.logs are changed to displayed correctly
This was done very well , great job!
Note edit 10-28-24 - I realized the complexity is greater than I thought at first only looking at the "medium" label. |
Added third main update to consider comments if the `created_at` date is recent (in order to avoid a minimizing of a comment being considered as an update)
Hi @codyyjxn FYI There is a last minute addition (noted by UPDATED 10/28/24 in the description above)- this happens at lines 108-109 in the |
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.
Hi Will thanks for working on this. I'm looking at the updates. All the changes look great. I noted down some stuff regarding how the workflow works.
The changes are great so I'll go ahead and approve them but still I had some questions regarding how the code works not the changes. The annotations made it very clear by the way but I want to be sure I understood it.
Feel free to correct any of the comments bellow.
Noted the change in variable twoMonthsAgo
for the months of September and February. Following the fact December and July are breaks. The refactoring of console.log() look is better.
Then I also saw the fact we needed to update the code for the new skills issue. That's the reason for the update from "Pre-work" to "Skills issue". Still the code works for "Pre-work checklist".
Looking into the way the code functions. We first get the data of the contributor's activity with get-contributors-data.js
and then we use that data to remove or notify inactive members using trim-inactive-member.js
The APIs return different values. We perform different checks to add the contributor to allContributorsSince
.
I still have to look at trim-inactive-member.js
in more detail. So the following might be answered there:
I have a question regarding the checks in ´get-contributors-data.js
. The "else/if" statements in the fetchContributors
function.
If a user appears as contributorInfo.author
or contributorInfo.user
then can their work be outdated? If so how is that checked?
I guess for both of them that is checked since the API call is since at maximum 2 months ago. For contributorInfo.user
we check for contributorInfo.created_at > date
so we are ok there.
Hey @santisecco thanks for the review and comments. For your question:
The To your question then, if an author or a user (or an assignee) name appears on the list, it is because they had activity since the date (one month or two months ago). I don't know if that helped or not- hope it did. Let me know if you have questions or want to talk about anything else |
Hey @codyyjxn I am merging this since you Approved this previously |
Thanks @t-will-gillis I'll look at it in more detail and ask you for anything else 👍 |
Fixes #7614
What changes did you make?
get-contributors-data.js
so that in the month after HfLA's break, theoneMonthAgo
variable remains at one month and is not adjusted for the time on break. Note thetwoMonthsAgo
does still account for the break monthcreated_at
date of the member's comment, and excludeupdated_at
date which is typically when a comment is hidden and is typically not performed by the membertrim-inactive-members.js
because the variablecurrentTeamMembers
is currently defined/ calculated in two places, unnecessarily. Also, streamlined theconsole.log()
for creating separation linesWhy did you make the changes (we will use this info to test)?
created_at
date rather thanupdated_at
dateScreenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)
Notes for PR reviewers
UPDATED 10/28/24: