-
-
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
Edit System Audit Text #4750
Edit System Audit Text #4750
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.
|
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.
LGTM
Thank you @Mayank2808sharma I am not sure how to test it, but I did run those commands in git... lol |
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.
LGTM
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 @DakuwoN Thank you for taking on this issue. This Pull Request looks good. The branches are setup correctly and the code change appears to be correct and clean. However to confirm the updated template is correct, I need the URL of the template in your test repository. Please edit Issue #4482, under "Link for Reviewer", replacing the text [REPLACE THIS TEXT AND BRACKETS WITH THE URL] with that URL. Then complete the test procedure outlined in this comment, by copying the listed sections from the Issue, into this PR, under the "What changes did you make..." section. Please send me a Slack message if you have any questions.
One other minor request: please provide a more descriptive name for the PR. The key point is that you have modified the labels on the "Design System Audit" template. Thanks
I explained the changes I made and why in the first comment above? Is this supposed to go elsewhere? |
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 @DakuwoN thank you for providing the URL. I looked and it looks fine. I just have two tiny requests:
Please update the name of the PR just to indicate that you updated labels on the "Edit System Audit: Text" template.
Please see the last Action Item in this comment. It is asking you to copy a few sections of text from the issue into the PR. After doing that please check off those last check boxes.
For PR Reviewers and Merge Team Link for Reviewers Once the pull request associated with this issue is approved and merged, please update and edit epic #4307 by |
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 @DakuwoN Thank you for making all of the requested changes. Due to the need to test the updated template, this was more involved than most of our good first issues and we really appreciate your attention and effort to complete all the details.
Fixes #4482
What changes did you make and why did you make them ?