Enhanced code to reference the mentioned labels by a general ID#7987
Enhanced code to reference the mentioned labels by a general ID#7987daras-cu merged 1 commit intohackforla:gh-pagesfrom
Conversation
- Imported retrieveLabelDirectory function - Added constant of the labels appearing in the file mapped to label-directory.json - Refactored the function postComment to use the constant variable
|
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. |
|
Hi @santisecco let me know when this is ready- thanks! |
|
Hi @t-will-gillis it's ready, thanks |
There was a problem hiding this comment.
Hey @santisecco Thanks for taking on this issue!
- Great explanations in the "What changes did you make?" and "Why did you make the changes"
- Fantastic instructions and accompanying screenshots in the "To reviewers" section- these notes are very helpful.
I tested in my repo and everything is working as intended, and the correct labels are being substituted in. The refactored postComment() function in check-label-preliminary-update.js is much more streamlined.
Great job!
|
@t-will-gillis Hi Will thank you! |
|
ETA: 3/21 EOD |
daras-cu
left a comment
There was a problem hiding this comment.
Hi @santisecco, thanks for working on this issue!
- Your branches are set up correctly
- The PR is clear and has a thorough explanation of what changes you made and why
- The provided screenshots and GH Actions logs from your testing were helpful to demonstrate that the labels were successfully retrieved
- The Issue Trigger action runs successfully when tested in my repo
Suggestions:
- In the original issue, one of the action items calls for you to check for references to a specific label name in the code comments and leave a comment with suggested changes. I see after your updates there are no such comments in the code, but you should still leave a comment on your issue confirming that you checked and no changes are required.
- It appears none of the issues you used for testing in your own repo had a role label included in
RELEVANT_ROLES, so thepostCommentfunction always returned false. If that's the case, I would go back and test if the results of the GH action are as expected when an issue does have one of these labels. That way you will be sure that the label names are being retrieved successfully from the directory, and there is no other issue with thepostCommentlogic.
|
Hi @daras-cu thanks for the comment. By the way, I just did a simple test file if you want to look at yourself test file As per your comment on the original issue, about the reference to a specific label, I didn't understand what that meant originally. I asked Will, the creator of the issue, and he mentioned that it was not necessary to do that. |
daras-cu
left a comment
There was a problem hiding this comment.
Hi @santisecco, thanks for trying the additional test issue with the role included, I got the same result when testing in my own repo so it looks like everything is working as it should be after your changes. Good idea to use the test file as well.
Noted about the comment action item, I'm glad you got clarification from Will. Overall great job on this issue!
Fixes #7533
What changes did you make?
retrieveLabelDirectory()function fromretrieve-label-directory.jsthat fetches the label value, also called labelName (thinking in terms of key-value pair).label-directory.jsonusingretrieveLabelDirectory()postComment()to use the constant variable created aboveWhy did you make the changes (we will use this info to test)?
retrieve-label-directory.jsmodule to look up the corresponding 'labelName' inlabel-directory.jsoninstead of hard-coding the labelNames.role: front end,role: back end/devOps,role: designandrole: user researchso the constant variable created addresses this labels in particular.CodeQL Alerts
After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.
Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown
Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.
Instructions for resolving CodeQL alerts
If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.
In general, CodeQL alerts should be resolved prior to PR reviews and merging
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)
To reviewers
console.log()to help you understand why the changes were performed that way and to see that the code works as intended.label-directory.json, this is fetched correctly. You must watch the raw logs of GHA for this, here goes a screenshot asretrieveLabelDirectory()logs this.Here are the screenshots
To see the raw logs open the Actions tab, and look for this
Here's the raw log of the last issue created that shows that the value from the directory is fetched correctly more clearly (as "front end" is now "front end GHA ACTIONS FETCHED CORRECTLY USING labelKey")