Added Sydney Walcoff to the expunge-assist.md File #5811
Conversation
…lly show up on my local environment
There was a problem hiding this comment.
@jleung7158 Thank you for taking up the good first issue! You wrote a nice pull request and you added Sydney info correctly in the _projects/expunge-assist.md. And Thank you for attaching before and after.
Only fix I ask is to put correct issue number. You put Fixes 5627 on pull request and branch name but I believe issue number is 5760.
|
Sorry about that @Tomomi-K1! I have fixed it now, let me know if there is anything else :] |
|
I'm not too sure if I properly changed the branch name though, as I am checking my git branches and my "update" doesn't appear. |
|
Review ETA: 6pm 10/31/23 |
|
Apparently I need to delete my old branch, remove this pull request, and make a new one. Is that alright? |
|
I forgot to put my availability and ETA. |
There was a problem hiding this comment.
Hi @jleung7158!
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. Sydney Walcoff's card has been added.
- Relevant images were included to show visual changes
- The PR request clearly states what was updated
- The PR request states why the changes are being made according to the original issue description
I do have a couple additional requests. When you're done, you can click the circle with the arrows next to my name in the reviewers section and I will approve the changes. Please feel free to ping me if you have any questions.
- @jackyuan1 noted the inconsistent issue number in the branch name. This can probably be fixed by changing the branch name. See these instructions for changing branch name. You may have to copy your PR into a new one when you push the branch again. If you have to create a new PR, please link this one and re-add the same reviewers.
- Please pull the changes into your local feature branch and push it to your forked repo again. There is currently a merge conflict because Samantha Hyler was added and that PR was merged before yours. This will update this PR with the changes.
- Add the labels that are on the original issue to this PR request (usually this happens automatically, but sometimes it doesn't work. It generally fails when the original issue number isn't added when the PR is created.)
Thanks for taking the time to contribute to the website!
There was a problem hiding this comment.
@jleung7158 , I see you fixed your issue number of this pull request!🙌🏻
-
Please kindly follow what @LRenDO mentioned in the review
-
Please make sure to update before and after screenshot. Currently at this moment, HackForLA website is showing below members.
If you pulled the main branch into your feature branch before you push out, you should see 6 people plus one more. But that could change as the things get merged. But you can always check Live HackForLA site to compare. -
It looks like you deleted Samantha Hyler and Curtis Barber. But you are supposed to add Sydney and not delete other members.
If you need any help, please let me know 😊!
|
I hope it's alright, I plan on just creating a new PR and linking this one! |
Fixes #5760
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)
Visuals before changes are applied
Visuals after changes are applied