-
-
Notifications
You must be signed in to change notification settings - Fork 778
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 Project Profile: Add Srishti Gupta to Expunge Assist Page #7757
Update Project Profile: Add Srishti Gupta to Expunge Assist Page #7757
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: 22 Nov 2024 @ 8 PM (GMT) |
Review ETA: Nov 22nd, 6 PM |
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 @katiejnete, thank you for your Pull Request!
Things that went well
- The Pull Request is done with the correct branch.
- The issue is linked well in the Pull Request.
- What changes you made and why are clear in the PR post.
- Your Availability and ETA are provided in the issue.
- The information for Srishti Gupta is added in the correct file as provided in the issue.
- Before and after screenshots show these changes.
- I can see these changes in my local environment.
- CodeQL alerts have been checked.
Things to change
-
Comments on the changes in the Pull Request: consider using backticks
(`)
to enclose the file path such as_projects/expunge-assist.md
. -
The correct slack for Srishti Gupta is https://hackforla.slack.com/team/U07FF4C4YFQ. Please update the code with this Slack URL. This also needs to be corrected in the original issue.
-
Try using http://localhost:4000 rather than 0.0.0.0:4000 for viewing the changes in your local environment.
-
The additional tabs in the provided screenshots are not needed. You can take the screenshot of the page in separate window or the specific visual change of the site.
Thank you for your contribution! Please feel free to ask if you have any questions. After making these changes re-request a review from me.
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.
Great job, @katiejnete!
- The PR is created with the correct branch.
- The branch name accurately describes the work done and includes the issue number.
- The issue is linked correctly.
- The "What" and "Why" sections are clear and concise.
- Before-and-after screenshots are provided to demonstrate the changes.
- CodeQL alerts have been checked.
Thank you for your contribution! Keep up the great work!
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.
Things that went well:
-Your pull request is done in the correct branch
-The issue is linked correctly
-You clearly understand the issue
-The changes are visible in the browser and images are clear
Things to Update:
- The correct slack for Srishti Gupta is https://hackforla.slack.com/team/U07FF4C4YFQ. Please update the code with this Slack URL. This also needs to be corrected in the original issue.
Review ETA: 23 Nov 2024 @ 10 AM (GMT) |
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.
@katiejnete Thank you for making the changes and re-requesting a review from me. Looks good! I approve this PR.
Great job @katiejnete with the PR and working with the reviewers to make the edits. |
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.
The links are all working.
Thank you @t-will-gillis |
Review ETA: 24 Nov 2024 @ 12am (CST) |
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 @katiejnete, thanks for the Pull Request!
Highlights:
The PR is targeting the correct branch.
The issue is properly linked.
Changes and their reasoning are clearly documented.
Availability and ETA are detailed in the issue.
Information for Srishti Gupta is added to the correct file.
Before/after screenshots effectively showcase the updates.
CodeQL alerts have been reviewed.
All links are working as expected.
Great work! 🎉
Review ETA: 25 Nov 2024 |
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.
@katiejnete Amazing job, all look good and also thank you for taking care of the changes point out by others.
Keep up the good work!!
Review ETA: 3:15 PM 11/25/2024 |
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, katiejnete!
You're doing these right:
- The branch you made the change on is correct
- The linked issue (original issue) number is included
- The content required to add is accurate
- The before / after screenshot is helpful to display the change
Great job!
Review ETA: 7:00pm PST 11/26/2024 |
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.
Hello @katiejnete
Branch name looks good.
The code changes look perfect.
The issue is linked correctly
CodeQL alerts are good
The content added is correct
Before and After photos look stunning.
Everything looks fantastic. Great job
Fixes #7703
What changes did you make?
_projects/expunge-assist.md
Why did you make the changes (we will use this info to test)?
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)
Visuals before changes are applied
Visuals after changes are applied