-
-
Notifications
You must be signed in to change notification settings - Fork 844
Update Project Profile Remove User 8070 #8092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Project Profile Remove User 8070 #8092
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. |
|
Availability: After 5pm |
|
Availability: After 5pm PST |
|
Availability: 8:30AM to 5PM PST Monday to Friday |
RodgerLugo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @valdezg ,
Great job! 👏
- Tested locally — confirmed that Nooria Ali was successfully removed from the Civic Tech Jobs project profile
- The site's appearance looks good, and the content is correct
- PR is made from the correct branch and links to the correct issue (#8070)
- The correct file (
_projects/civic-tech-jobs.md) was edited with no extra changes - Everything looks clean, and screenshots are clear
Just a small suggestion for next time: consider including the person's name in the PR title (e.g., "Update Project Profile: Remove Nooria Ali – #8070") so it's easier to identify.
Good job — keep up the great work! 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @valdezg! Thank you for taking on this issue!
Things Done Well
- The issue has the correct 'commit into': "hackforla:gh-pages".
- 'commit from' and 'collaborator' names match
- The linked issue is present
- The changes made are clean.
- Changes were viewable in browser.
Suggestions
I have no suggestions of my own, but do agree with @RodgerLugo 's suggestion to consider adding the name, rather than the issue number in the PR title. Before I approve the changes, however, I am also curious about the nature of the added 'vrms.md' file for 'Files changed' that @hoanhua14 commented on, as I don't see any mention of it in the issue description. Some clarification on that would be much appreciated.
Other than that, great job working on this issue!
hoanhua14
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Hi @xnealcarson @xnealcarson @RodgerLugo So I updated the VRMS.md file in a previous pull request #8046 which is why I think its accidentally being pushed in this commit. I am not sure what steps I need to take to remove it. I am considering closing this PR and resubmitting because I am not sure why that file was included. Any advice @t-will-gillis @daras-cu ?? |
|
Hi @valdezg From what you are saying about the changes from the previous PR accidentally being included here: the best way to avoid this is to make sure to delete the branch(es) from the previously merged PR(s), and then sync As you mention, at this time it will be easiest to close this PR, delete any old branches from previously merged PRs (CONTRIBUTING.md , and then sync When you close this PR, please briefly explain your reason in the comments and link to the new PR. You can request me and the current reviewers to review the new PR. |
Fixes #8070
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
Visuals after changes are applied