-
-
Notifications
You must be signed in to change notification settings - Fork 802
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
Added Angela Lee to VRMS leadership, with corrected links #7981
base: gh-pages
Are you sure you want to change the base?
Added Angela Lee to VRMS leadership, with corrected links #7981
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: 5PM March 8, 2025 |
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.
Looks good @Crocodile-Lyle !
- The pull request is made with the correct branch
- The issue is correctly linked
- Angela has been added to projects/vrms
- Before and after images match my local and prod environments
Questions & Suggestions:
- I've left two comments about differences between the text in the referring issue and the updated text in your PR. My local environment links to the correct github handle and displays the github profile's image correctly, however I'm wondering if there was a discussion elsewhere about the change in text. I noticed that the PR indicates 'corrected links' in the title, but I feel like I've missed a step in the process!
- The branch name could be a little more descriptive i.e. added-angela-lee-to-vrms-profile
Hello! Yes, I should have documented what I did more clearly. The links to the github profile as well as the photo that were provided on the issue were not valid URLs. I had to find her profile on github and paste the correct URLs for the profile and photo. This is because her profile lives at 'github.com/angela-lee1/awlFCCamp' not 'github.com/awlFCCamp'. Next time I will document my steps more clearly rather than just use a vague title. Thank you for your review! |
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.
@Crocodile-Lyle thanks for you feedback!
Approved! :)
Availability: Monday - Friday, 10:00 AM to 4:00 PM JST |
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 @Crocodile-Lyle !
Great job with this pull request!
- The branching was done correctly.
- Issue number was listed.
- The PR title is descriptive of the changes.
- Changes were made correctly in the code.
- Changes appear correctly on the site - Angela Lee was Added from VRMS page.
- Relevant images were included to show visual changes.
- The PR request clearly states what was updated.
Again, great work and thanks for taking the time to contribute to the website!
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 @Crocodile-Lyle Thanks for working on this issue. A couple of notes:
-
It is great that you (and @DDVVPP) noticed the discrepancy in the entry for the user. But please remember that if you see a disprepancy, it is always safest to contact the issue writer or and/or the someone on the merge team to discuss whether changes need to be made. If so you need to note the changes on the PR.
-
In this case, the developer switched their GitHub handle from
awlfccamp
and they are now usingangela-lee1
as their primary handle so this is what we want to use also. -
Also, although the
picture
link you provided works for this person, the link format shown below is the format we use in these entries. (And it also works of course.) -
Therefore, please correct the entry as follows:
- name: Angela Lee github-handle: angela-lee1 role: Developer links: slack: https://hackforla.slack.com/team/U062NB2CKMZ github: https://github.com/angela-lee1/ picture: https://avatars.githubusercontent.com/angela-lee1
-
Also note that a separate PR was merged while you were working on this one, and that is causing a merge conflict that needs to be resolved. It is relatively easy to address- just need to keep the new entry for Mudassir Hussain. See the instructions below and let me know if you need any help with it.
Thanks- when you are done making changes, please select to 'Re-request review' for the three current PR reviewers.
bd478f0
@t-will-gillis Ok good info, thank you! I just tried to use the online GUI to resolve the merge conflict. Can you let me know if it looks good now? |
Hi @Crocodile-Lyle Yes- looks like the merge conflict is resolved. |
Hello @Crocodile-Lyle, are you ok? I checked your task again, and the conflict was resolved correctly, but you didn't make the requested change. Did you see that the issue was updated? You need to replace awlfccamp with angela-lee1 in several places, not just in the GitHub link. Make sure to use the correct data provided by @t-will-gillis.
Observe the differences in your branch: If you need help, you can reach out to me on Slack. :) |
Fixes #7877
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
Before:

Visuals after changes are applied
After:
