Skip to content
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

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from

Conversation

acterin
Copy link
Member

@acterin acterin commented Jan 29, 2025

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

Why did you make the changes (we will use this info to test)?

  • made the changes because they were requested in the git issue

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

Screenshot 2024-10-28 154514

Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.

  • I have checked this PR for CodeQL alerts and none were found.
  • I found CodeQL alert(s), and (select one):
    • I have resolved the CodeQL alert(s) as noted
    • I believe the CodeQL alert(s) is a false positive (Merge Team will evaluate)
    • I have followed the Instructions below, but I am still stuck (Merge Team will evaluate)
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

Visuals after changes are applied

After image

Copy link

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.

git checkout -b acterin-acterin/issue7828 gh-pages
git pull https://github.com/acterin/website.git acterin/issue7828

@github-actions github-actions bot added good first issue Good for newcomers role: front end Tasks for front end developers role: back end/devOps Tasks for back-end developers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) time sensitive Needs to be worked on by a particular timeframe size: 0.25pt Can be done in 0.5 to 1.5 hours labels Jan 29, 2025
@acterin acterin requested a review from Cloid January 29, 2025 06:54
@Cloid
Copy link
Member

Cloid commented Jan 30, 2025

Review ETA: 5 PM 1/30/25
Availability: 5-8 PM Weekdays

Copy link
Member

@Cloid Cloid left a 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.

Click to view image (live website)

Sample Image

@acterin acterin changed the title Acterin/issue7828 Acterin/Update Project Profile: Website Add Sofiat Ajide Jan 31, 2025
@acterin acterin changed the title Acterin/Update Project Profile: Website Add Sofiat Ajide Updated Project Profile: Website Add Sofiat Ajide Jan 31, 2025
@acterin
Copy link
Member Author

acterin commented Jan 31, 2025

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.
Click to view image (live website)

Hi @Cloid, I have made your suggested change and changed the title from a vague issue # to "Update Project Profile: Website Add Sofiat Ajide."
Thank you for reviewing my pull request!

@acterin acterin requested a review from Cloid January 31, 2025 04:56
Copy link
Member

@Cloid Cloid left a 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

@DakuwoN
Copy link
Member

DakuwoN commented Jan 31, 2025

Review ETA: 2/2/25
Availability: Monday - Sunday 1PM - 7PM Central

Copy link
Member

@DakuwoN DakuwoN left a 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!

@t-will-gillis t-will-gillis self-requested a review February 2, 2025 22:40
Copy link
Member

@t-will-gillis t-will-gillis left a 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!

To re-request reviews

Screenshot 2025-02-02 150513

@t-will-gillis
Copy link
Member

@katiejnete
Copy link
Member

Review ETA: EOD 2/7/25
Availability: 7-9PM 2/7/25

Copy link
Member

@katiejnete katiejnete left a 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:

After making these changes, re-request a review from me. Thank you for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) role: back end/devOps Tasks for back-end developers role: front end Tasks for front end developers size: 0.25pt Can be done in 0.5 to 1.5 hours time sensitive Needs to be worked on by a particular timeframe
Projects
Status: PRs being reviewed
Development

Successfully merging this pull request may close these issues.

Update Project Profile: Website Add Sofiat Ajide
5 participants