added github-handle to Emily Eldar #7227
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. |
github-actions
bot
added
good first issue
|
Review ETA: 4 PM 8/8/24 |
There was a problem hiding this comment.
Good job with this pull request! The "to" and "from" branches are set up correctly, the relevant issue is linked correctly, and the necessary change has been successfully made.
A few changes I would like to request:
- I see that you made additional edits to _projects/home-unite-us.md. What was the motivation behind these edits? Even if they are necessary, they should most likely be part of a different pull request.
- If there are no visual changes made to the website, please do not screen shot code changes. Instead, delete the image section altogether and add a small explanation as to why there are no visual changes. You can say "- No visual changes to the website"
- You don't need to specify "good first issue" as a reason for why you made the changes.
Thank you for your time and contributions!
There was a problem hiding this comment.
Hey @pluto-bell Great job on your first HfLA PR.
- Branches will be ok. For the next time, please include the issue number with the name of your branch, similarly to what is shown in
CONTRIBUTING.mdSect. 2.7.b. In other words, your branch should be named something like:add-github-handle-7191. - The sections for what was changed and why are fine. You have provided brief, relevant answers here. You can remove the bullet for "- First good issue."
- For the "Screenshots" section, please remove the dropdown images after your comment "No visual changes were made to the website."
- The code changes are good except as @TheManTheMythTheGameDev mentions, you don't want to go outside the scope of the issue. In the future, the best thing to do when you see things like this will be to bring it up with the issue writer and/or the merge team. Sometimes there is another issue for addressing what you see and sometimes there will be a good reason that the code is written in some way. (Confession- I have been guilty of making little edits like this also...)
Thank you for working on this!
Returned double-quotes to single quotes.
|
@t-will-gillis @TheManTheMythTheGameDev, just want to be clear on what happened with those extra changes. I didn't recall touching anything else so I did some digging and came to realize both line 6 and 114 got modified by some type of code formatter (akin to VS Code's "Prettier" extension). I disabled my extensions so that shouldn't occur moving forward. I returned the alt text to single quotes, instead of the modified double. |
|
Hi @pluto-bell Thank you for making the edits. Understand about the auto-formatting. Yes, please change the url line 114 back to single quotes also. |
|
@t-will-gillis great, I've finalized the edits (: Should be good to go now! |
Hi @TheManTheMythTheGameDev, I think this merge is currently waiting on your review? As it still says "Changes Requested" from you. I believe everything should be good to go but let me know if not. |
|
@perlaroyerc Please add a comment with your availability and ETA for this PR review |
There was a problem hiding this comment.
Hi @pluto-bell! Yes, everything looks good to me now. Sorry if I wasn't clear about which double quotes should be returned to single quotes, but it seems as if that has all been resolved. The description looks good too. Good work on this issue, approved!
* added github-handle to Emily Eldar * Update home-unite-us.md Returned double-quotes to single quotes. * return to single quotes
Fixes #7191
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)
No visual changes were made to the website.