Updated Virginia Wu's section by adding "github-handle"#6991
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. |
|
My mistake, I just realized I have the "Prettier" extension and it automatically formatted some other parts of the code (removed extraneous spaces and changed double quotes to single quotes). Let me know if this needs to be fixed, I'll be sure to disable it for future issues. |
|
Review ETA: EOD 6/12/24 |
|
Hi @cchrizzle, nice job working on this issue! Everything looks good on my end. Regarding the Prettier automatic formatting that occurred, I think it's alright, but we should confirm with the tech leads. @roslynwythe, I would love to get your opinion on this. I'll hold off on approving the pull request until we get the green light from the tech leads about whether the Prettier formatting is acceptable. Other than that, nice job! Here’s what you did well:
|
|
Review ETA: EOD 6/12/24 |
andyphancode
left a comment
There was a problem hiding this comment.
Went ahead and took a look at all the changes and they are consistent with the explanation.
- Correct branch usage
- Correct addition of
github-handle: - PR comment is complete
I agree with @Anahisv23 's take on the situation but it is out of my scope to say if it is wrong or right. I can give the PR my approval when someone with more authority comments on the situation or you can take the safer, quicker route of re-doing the pull request with the extension off or making another commit fixing all the changes that are not necessary.
For future reference I recommend checking what lines have changed either by looking at your Git GUI you have installed or using git show in your terminal. @cchrizzle
|
I just resubmitted with the original formatting intact, so the only change from the original file is the addition of "github-handle" for Virginia Wu's section as originally requested. (Also I just realized I put "Victoria" instead of "Virginia" in the pull request title and just fixed that as well!) @Anahisv23 I appreciate your time and input, thank you! @andyphancode Followed your advice with the resubmission, thanks for letting me know about |
Anahisv23
left a comment
There was a problem hiding this comment.
Hi @cchrizzle thanks for fixing the prettier formatting issue and for updating the name from "Victoria" to "Virginia" on the PR title. @andyphancode this was a great suggestion. These new code changes look great. Approved!
ajb176
left a comment
There was a problem hiding this comment.
Hey @cchrizzle, nice work.
Yes, you should avoid using Prettier when working on issues, VSCode + Spell checker is the standard, you should be able to disable Prettier in your VSCode workspace.
Your PR looks good, one thing to keep in mind for next time is that the 'why did you make the changes' section should be a brief, high level description of what the PR is meant to address. The overview section of the linked issue mentions this, which would be a decent answer:
We need to create a single variable github-handle to hold the github handle for each member of the leadership team. Eventually github-handle will replace the github and picture variables, reducing redundancy in the project file.
Just something like 'To reduce redundancy in the project file' or however you want to phrase it would be good enough.
|
@Anahisv23 thanks again, much appreciated! @ajb176 got it, disabled Prettier and will continue like this from now on. Thank you so much for the suggestion on how to have better descriptions, it was really helpful to help me get a better idea of what is expected and I've updated my pull request accordingly. |
del9ra
left a comment
There was a problem hiding this comment.
Hi @cchrizzle,
- The pull request is done with the correct branch
- The issue is properly linked
- Your summary of changes is clear
- Your code change correctly uses spaces instead of tabs to indent
- The appearance of the project webpage is unchanged
Well done!
Thanks for your contribution
Fixes #6737
What changes did you make?
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Updated markdown file - no visual changes to the website.