Add Audreen Fune to 311 Data Page#8243
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: 6 PM 7/14/2025 |
There was a problem hiding this comment.
Hi @nubilaxl Congratulations on submitting your first PR! Looks great:
- the 'from' and 'to' branches are correct
- the original issue is linked
- there are before and after screenshots attached
- the code changes appear to be correct
Here are some comments:
- For "What changes did you make?", you can answer this more concisely by saying "Added Audreen Fune to 311 Data project page" or something similar
- The "Why did you make the changes..." response is good
- Please check the appropriate box for the "CodeQL Alerts" section, that is "I have checked this PR for CodeQL alerts and none were found"
- Finally, if you scroll to the bottom of this PR you will see that your branch has a merge conflict that needs to be resolved. Merge conflicts happen when multiple people work on the same file. In this case, there were two related PRs that were merged after you started working on this- Holly Worthen and Allison Jeon were both removed. You can either select the "Resolve conflicts" button below and manually edit the file so only Audreen's profile remains- delete the other two profiles as well as the "<<<<<<< add-audreen...", "======", ">>>>>> gh-pages"- and then commit the changes. Or you can also go to the your branch in your own repo and select "Sync fork". Either way if you have trouble let us know.
Thank you for working on this and great job again.
santi-jose
left a comment
There was a problem hiding this comment.
Hi @nubilaxl ! Great job submitting your first PR. It looks good!
- The title gives information at a glance to what issue the PR is addressing
- The into and from branches are correct
- Before and after pictures are included
- The code added to the file matches what is requested in the Issue
A couple changes I second from @t-will-gillis review:
- Check your PR for CodeQL alerts. These annotate any code which need review after you submit your PR. If you have any CodeQL alerts make sure to mark it accordingly within your PR in the checkbox section titled "CodeQL Alerts"
- Resolve merge conflicts in your feature branch add-audreen-fune-8014. As noted by @t-will-gillis , Holly Worthen and Allison Jeon were removed from the main branch after your PR was submitted. You need to resolve these merge conflicts. You can simply remove these two from the 311-data.md file in your feature branch, or I believe syncing your feature branch and pulling the latest changes would also fix this issue. Note that this may also remove the changes you made s.a. adding Audreen Fune to the 311-data leadership property. Therefore it may be simpler to resolve the merge conflicts one at a time by removing Holly Worthen and Allison Jeon from the 311 leadership property.
All in all, very solid work for your first PR. When you get the chance to make these changes I'll make sure to review again!
t-will-gillis
left a comment
There was a problem hiding this comment.
Hi @nubilaxl Thank you for resolving the merge conflict- looks good!
A couple minor notes-
- For the "What changes did you make?" you can shorten this to
The rest of the info is not needed. Just copy and insert the note above (with the dash at the start as shown.
- Added Audreen Fune to 311 Data project page - On the original issue 8014, please check the boxes on the items you completed for the "Action Items"
Almost there - when you make these changes, be sure to re-request a review from both of us so that we know to look out for it.
Thanks for sticking with this!
t-will-gillis
left a comment
There was a problem hiding this comment.
Hey @nubilaxl Perfect- thanks so much
myronchen-git
left a comment
There was a problem hiding this comment.
Good job.
- The merge from and to branches are correct.
- The issue is linked correctly.
- The pull request info is filled out and has relevant screenshots.
Fixes #8014
What changes did you make?
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