-
-
Notifications
You must be signed in to change notification settings - Fork 791
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
Updated Project Profile: Website Add Sofiat Ajide #7862
base: gh-pages
Are you sure you want to change the base?
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.
|
Review ETA: 5 PM 1/30/25 |
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 @acterin,
I read Issue #7753 and tested the changes in this PR. Everything looks great in terms of code and the changes are reflected in the site.
The only change I would make is the Pull Request name to be changed from issue7828 to Update Project Profile: Website Add Sofiat Ajide. The current name is vague and the issue of the branch does not exist or relate to the issue being referenced. Just change the name and I'll approve it.
Great job still on this pull request!
side note for @DrAcula27 or other reviewers:
When I look at the live website, there are two Product Managers listed at the bottom which isn't consistent with this Issue's Product Manager placement. See image dropdown below and Issue #7619 / #7751.
Hi @Cloid, I have made your suggested change and changed the title from a vague issue # to "Update Project Profile: Website Add Sofiat Ajide." |
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.
Thanks for the change. Approving this PR
Review ETA: 2/2/25 |
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.
Everything looks fine in terms of the review! Great job @acterin and thank you so much for the contribution!
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 @acterin - looking good, some comments:
- The branch name is not explicitly wrong, however we typically name branches referencing the original issue and the issue number, so for this one a typical branch name would be:
add-sofiat-ajide-7828
. No need to change this one. - The template comment starting "Please note: you must be a member..." at the top should be deleted from the PR
- The line for "(Update Project Profile: Website Add Sofiat Ajide # 7753)" should be deleted
- Regarding the "What changes did you make?", the answer is intended to be a short, concise summary- for this issue the answer would be "Add Sofiat Ajide profile to Website Project" or similar- rather than stating exactly what you did. Please change this.
- Regarding "Why did you make the changes...", the answer is the reason for the issue itself, not because the issue said to. (All PRs are done because there is an issue describing what needs to be done). The answer to this question is in the "Overview" of the original issue, so the answer should be something like: "To keep the website project info up to date." Please change this.
- One final thing: in the time that you were working on this issue, another related issue (to add "Eleftherios Christou") was merged so now there is a merge conflict. This happens sometimes. What you will need to do is scroll down to the bottom of the issue, select "Resolve conflicts", edit the file where indicated to accept Eleftherios's profile, then "Mark as resolved".
Please makes the changes noted, and when you are done re-request reviews from myself, @DakuwoN , and @Cloid. Thanks for working on this!
|
Review ETA: EOD 2/7/25 |
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 @acterin , thank you for taking on this issue.
Things you did well:
- PR is done with the correct branch.
- CodeQL Alerts have been properly checked.
- Before and after screenshots are appropriately included.
- The changes are applicable to the issue.
- Website is still user-friendly and links and components still work as intended.
- Source code changes are applicable and clean.
Suggested changes:
- You can delete the “must be a member of the HFLA…” paragraph at the top of this PR.
- You should reference the following template for your PR, remove unnecessary lines, and explain changes made appropriately (explanations can be copied from the linked issue); for example,
Fixes #7753
What changes did you make?
- Updated
_projects/website.md
markdown file and added new profile after
Bonnie, after “Product Owner” entries, and before “Developer Co-leads” entries.Why did you make the changes (we will use this info to test)?
- To keep list of project profiles for the project team up-to-date.
- Please resolve the merge conflict.
Notes:
- In the future, your issue branch name should relate to the issue and follow the correct scheme/format. For example, the issue Update ‘Give’ image credit link and information - #2093 should have this branch name:
update-give-link-2093
. In your case, your issue branch name should be for #7753.
After making these changes, re-request a review from me. Thank you for your hard work!
Please note: You must be a member of the HFLA website team in order to create pull requests. Please see our page on how to join us as a member at HFLA: https://www.hackforla.org/getting-started. Delete this message if you joined this team via onboarding.
Fixes #7753
(Update Project Profile: Website Add Sofiat Ajide #7753)
What changes did you make?
made changes to lines 22-28
github-handle: sofiatajide
role: Product Manager - Dashboards
links:
slack: https://hackforla.slack.com/team/U07LRE68BHS
github: https://github.com/sofiatajide
picture: https://avatars.githubusercontent.com/sofiatajide
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
The bottom toolbar is a my local browser stuff. Please ignore it.
![Before image](https://camo.githubusercontent.com/1c6d2af1fec638adc6a5338b12945abf3989ece34737c1fd8e7aaea19d2b35a3/68747470733a2f2f692e696d6775722e636f6d2f537a39474a574f2e706e67)
Visuals after changes are applied