-
-
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
Update spell-check-audit.md label and issue template #7713
Update spell-check-audit.md label and issue template #7713
Conversation
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.
|
Review ETA: 11/11/2024 Monday |
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.
Thank you for working on this issue, @aswutmaxcy .
- The branch you made change on is correct
- The linked issue number is included
- Availability and ETA are provided in the original issue
- The change you made in the related file is accurate
Improvements:
- Address the question
Why did you make the changes (we will use this info to test)?
with specific detail based on the original issue - Respond to
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)
, provide related screenshots if any, otherwise, keep the answerNo visual changes to the website
from your PR template
Reminder:
- After PR is merged, be sure to change your default branch back to gh-pages and to disable Issues feature
Changes you need to make:
- Fix the labels in the issue template you linked in the For Reviewers section (there aren't any labels showing up)
Aren't the labels specific to the repo? As in, I won't have them on my clone of the repo, but they'll show up on the main hfla repo |
Review completion by: Nov. 14 |
Review ETA: End of day Thursday, Nov 14 |
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.
To: @aswutmaxcy
Comments: I see that the corrections were made in the correct branch, and the link to the issue has been added in the comments. Also, the corrections related to the issue have been applied.
Corrections/Suggestions: I don’t see screenshots of the changes in the comments. Please add these to provide visual confirmation of the updates. Maybe just a screenshot of the spell-check-audit.md page. Thank you!
@siyunfeng can you please respond to my question about the labels |
Hi @aswutmaxcy , since the issue template involves changing the labels, we should make sure all the correct labels show up in the test issue template on your branch. |
@siyunfeng the issue doesn't involve changing labels, but the text in the template. You can create labels to show up, but that isn't testing the labels at all, it would just be me creating labels for show. |
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.
@aswutmaxcy Great work on this. I can see that you've replaced dev lead with merge team in the two required places.
As mentioned by @siyunfeng, I think it would be worthwhile to know that the labels will appear as expected before merging this, if possible. However, I don't fully understand the flow of how these comma-separated labels turn into issue labels. Is this controlled by a GitHub action, or is it a default functionality of GitHub? Is there a feasible way to test this in a personal repo?
@tamara-snyder yeah, that's what I'm trying to figure out too, I can't get them to come up naturally unless I just manually add them on my repo (which doesn't test anything, it'd just be for show). I'm guessing the labels are controlled by the repo owners or people with the access to write/edit labels on the actual repo. Which leads me to believe labels are repo specific, unsure though... All I know is I can't get them to come up on my side. |
I confirmed with @roslynwythe in the meeting. The issue instruction is missing the steps of creating labels on your branch. To create the labels, I recommend using this Testing issue templates as reference:
Once you create all the labels listed in the file |
@siyunfeng okay, got the labels in, the url should be updated and good to go, lemme know if it shows up on your side with the labels as well |
@jennisung I added a screenshot of the issue template page as well |
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.
UPDATED Comments: I see that the corrections were made in the correct branch, and the link to the issue has been added in the comments. Also, the corrections related to the issue have been applied. Assignee added the screenshots of the changes in the comments that showed that he corrected the .md page with the correct wording.
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.
Nice job adding the labels to the issue template, @aswutmaxcy .
- I can see the labels in the updated issue template match those listed in the file
/.github/ISSUE_TEMPLATE/spell-check-audit.md
. - The screenshot helps show the changes
Things to improve:
- You can refer to the
Overview
section from the original issue Replace label in spell-check-audit.md #7601 to address the questionWhy did you make the changes (we will use this info to test)?
.
Reminder:
- After PR is merged, be sure to change your default branch back to
gh-pages
and disable theIssues
feature
Thank you for your contribution! Keep it up.
Fixes #7601
What changes did you make?
Why did you make the changes (we will use this info to test)?
For Reviewers
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)