added github handle for Priyanka Talwar - not Bryanna#7369
added github handle for Priyanka Talwar - not Bryanna#7369jphamtv merged 1 commit intohackforla:gh-pagesfrom
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: 8/30/24 EOD |
|
Branches look good, correct addition to Priyanka. Good catch on the inconsistency of names. @roslynwythe did they select the proper individual to add github-handle to, or was the issue intended to change for Bryanna Lim? For @NolaDodd , please add a "Why did you make the changes (we will use this info to test)?" regularly to pull requests. A good idea would be "Added a github handle label to hold github profile link of person" or whatever you see appropriate. |
Added the "Why" for the request, thank you! |
|
Hi @NolaDodd (and @andyphancode , @roslynwythe ) Thanks for pointing out the mix up. The issue title is now updated to #7248, and also updated the issue and issue title for #7239 to address "Bryanna Lin" |
|
Review ETA: EOD Sep 4, 2024 |
8alpreet
left a comment
There was a problem hiding this comment.
Hi @NolaDodd,
What went well:
- Correct lines of code were changed.
- PR title is descriptive and concise.
- Branch is named correctly.
- Related issue is linked.
Changed Needed:
- Why section:
- your why section is very similar to your what section because it describes what you did not why you did it. I know the distinction is blurry on these good first issue, but the why is the cause of the changes. Let's say for example, that some elements in the website were having weird sizing issue on mobile devices. You figure out that some CSS in file X is the cause. So your what would be: I changed _____ in file X. Your why should explain why it was those lines of code that needed to be changed.
- Screenshots section:
- you don't need the quotes around "- No visual changes..."
k-cardon
left a comment
There was a problem hiding this comment.
This looks correct, @NolaDodd! You branched correctly, added the correct lines of code, used spaces instead of indent, and I confirmed no visual changes to site.
Since Balpreet wants a change to the "why" section of the pull request, you could pull text from the issue, which explains why the site is updating the github-handle variable:
"Eventually github-handle will replace the github and picture variables, reducing redundancy in the project file."
|
@NolaDodd Please make requested changes and re-request a review. |
8alpreet
left a comment
There was a problem hiding this comment.
Things look great now, thanks for applying the feedback!
Fixes #7248
What changes did you make?
UPDATE*** Merge Team changed title to reference "Priyanka Talwar"
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)