Home unite us remove muyin zheng #8071
Conversation
github-project-automation
bot
moved this to PR Needs review
in P: HfLA Website: Project Board
|
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
andyvu923
left a comment
andyvu923
left a comment
There was a problem hiding this comment.
Hi @michael-4 , thanks for working on this issue!
Things done well
- Correct branches are used
- PR links correct issue
- Before and after pictures are added
Suggested Changes
It seems that your fork is not synced with the main repo. There are some additions in the file that have been removed by previous closed PRs that should not be added back. These additions should be removed from the PR.
Once these changes are completed, please re-request a review from me. Thank you!
github-project-automation
bot
moved this from PR Needs review
to PRs being reviewed
in P: HfLA Website: Project Board
| slack: "https://hackforla.slack.com/team/ULN1M6UAH" | ||
| github: "https://github.com/tylerthome" | ||
| picture: https://avatars.githubusercontent.com/tylerthome | ||
| - name: Jed Stewart |
_projects/home-unite-us.md
Outdated
| picture: https://avatars.githubusercontent.com/rpbracker | ||
| - name: Muyin Zheng | ||
| github-handle: | ||
| - name: Samuel Kowitch |
_projects/home-unite-us.md
Outdated
| slack: "https://hackforla.slack.com/team/U04GYSFB98X" | ||
| github: "https://github.com/KowDesign" | ||
| picture: https://avatars.githubusercontent.com/KowDesign | ||
| - name: Emily Eldar |
|
Availability: after 1pm (pacific) |
|
Availability: After 5pm PST |
remove muyin-zheng from home-unite-us :wq
michael-4
force-pushed
the
home-unite-us-remove-muyin-zheng-7483
branch
from
f08a354 to
1179c0b
Compare
|
I rebased onto the latest gh-pages and resolved the merge conflicts. Ready for review 👍 |
xnealcarson
left a comment
xnealcarson
left a comment
There was a problem hiding this comment.
Hi @michael-4! Thank you for tackling this issue.
Things You Did Well
- The issue has the correct 'commit into'
- the 'commit from' and 'collaborator' names match
- The linked issue is present: 'Fixes #7483'.
- Changes were viewable in browser.
Suggestions
When viewing your changes in the browser, I noticed the additions in the file that @andyvu923 mentioned are still present there and they are still present in the source code too. Besides that, I have no requested changes or suggestions of my own. So once you've removed those additions in the file, I can go ahead and approve your PR!
There was a problem hiding this comment.
Way to go, Michael!
Things Done Well
- The pull request done with the correct branch.
- There's a linked issue, and I understand it.
- I took a look at files changed tab.
- I viewed the changes in the browser, see everything's been deleted, and took a screenshot.
Suggestions, Comments, Etc.
This is more of a comment. Lines 39-40 seem to be part of Muyin Zheng's info, but lines 39-40 look as if they haven't been deleted. I didn't notice anything odd in the code I checked on your branch in my IDE or when checking the page in my browser, but I didn't want to not say anything about it.
andyvu923
left a comment
andyvu923
left a comment
There was a problem hiding this comment.
Hi @michael-4 , thanks again for working on this issue.
I ran the code on my browser and this is currently what I have:
This is what I have when running on my fork's gh-pages branch:
It seems that your current working branch might still be outdated. I checked your forked repo, and it seems to be a couple commits behind. I think syncing the fork to the main repo would help solve this issue.
As always, thanks for the hard work!
|
Okay I believe I've synced with the main repo correctly this time. Although a new person appeared "Ariel Lasry" which isn't in either of the screenshots above. I'm not sure if this person was added in the meantime or if I'm still out of sync somehow so please let me know! 
|
|
Looks good! The only change left I would suggest is to make sure to remove the changes adding Jed Stewart back, and after that everything looks good to me. Thank you! |
|
Hi @michael-4 From the "Files changed", it looks like your commit is trying to add "Jed Stewart" back after being deleted by another PR. This happens when you are not starting with an up to date branch. At this point, the easiest way to fix this will be to close this PR, delete the PR branch, and start over making sure that your When you close this PR, provide a brief reason and then link the number for the replacement PR. Request reviews from me and the current reviewers. Thanks |
|
Closing this PR because it was created from an outdated branch and unintentionally reintroduced previously deleted content. A new pull request has been created which has #8127 |
5 participants
Fixes #7483
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
![image]
Visuals after changes are applied
![image]
