Skip to content

Conversation

@praytoo
Copy link
Member

@praytoo praytoo commented Jun 19, 2025

Fixes #8010

What changes did you make?

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

  • Kristen Cardon was no longer needed in the leadership section

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

image
Screen Shot 2025-06-24 at 11 17 23 AM

Visuals after changes are applied

image
Screen Shot 2025-06-24 at 2 56 08 PM

@github-actions
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 praytoo-remove-kristen-cardon gh-pages
git pull https://github.com/praytoo/website.git remove-kristen-cardon

@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 Jun 19, 2025
@FatCatLikesBeer FatCatLikesBeer self-requested a review June 19, 2025 06:50
@FatCatLikesBeer
Copy link
Member

Availability: Mon-Sat after 6pm PST
ETA: 6/20/2025

@caz002 caz002 self-requested a review June 19, 2025 21:34
Copy link
Member

@FatCatLikesBeer FatCatLikesBeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @praytoo ! Thank you for taking on this issue!


Things Done Well

  • The pull request done with the correct branch.
  • Issue is linked and understandable.
  • Appropriate changes in the Files Changed tab.
  • The changes can be seen in the browser.

Requested Changes

  • CodeQL Alerts have not been properly checked. Please check for CodeQL Alerts (near the bottom of this page), then select the appropriate checkbox in your pull request post.
  • Screenshots: please include screenshots in the bottom of your pull request post. A gif demonstration can be found in 3.1.b in CONTRIBUTING.md.

Notes:

  • I really like your use of backticks in the pull request comment, it really helps clarify that your referencing files or project code. I'm need to start using that myself!

Once you've made these changes, go ahead a re-request a review from me. Thank you for your hard work! Don't hesitate to reach out if you have any questions.

@github-project-automation github-project-automation bot moved this from PR Needs review to PRs being reviewed in P: HfLA Website: Project Board Jun 20, 2025
@praytoo
Copy link
Member Author

praytoo commented Jun 20, 2025

Hi @FatCatLikesBeer 👋 Thanks again for the feedback!

  • I have reviewed the CodeQL Alerts and confirmed there are no issues.
  • Added requested screenshot(s) for visual confirmation.

✅ Visual Confirmation

Before

Screen Shot 2025-06-18 at 8 01 23 PM

After

Kristen Cardon has been removed from the leadership section as intended.
Screen Shot 2025-06-18 at 8 04 47 PM

@praytoo praytoo requested a review from FatCatLikesBeer June 20, 2025 16:41
@ryanfkeller ryanfkeller mentioned this pull request Jun 20, 2025
5 tasks
@ryanfkeller
Copy link
Member

ryanfkeller commented Jun 20, 2025

Hi @praytoo -- I think @FatCatLikesBeer is saying you should include screen caps of the visual changes to the website, not snips of changes to your code. Also, the screen captures should be included as part of the main PR description rather than in a comment.

You can see on the currently hosted version of the HfLA website project page that Kristen's card is present under project leadership. With your change, Kristen's card should be removed, and we want the PR description to show the before/after. To be able to see the effect of your code change before it is integrated into the official HfLA codebase, you will need to serve the page locally on your machine -- steps to do that are in CONTRIBUTING.md. If you can do that, you'll be able to confirm your changes work locally before they're merged, and grab before/after screenshots easily

Please reach out if you need any help!

@FatCatLikesBeer
Copy link
Member

Hi @praytoo , thank you for the quick turn around!

A couple clarifications, in my previous comment, I made a mistake when saying your "pull request comment". I should have said your "pull request post". I've updated my comment to reflect that.

The expected screenshots are of the website itself, not screenshots of the code. An example can be found here #8116 at the bottom the post (click the little triangle thingies).

Again, thank you for the quick response, and thank you for your hard work! 💪🏽

@praytoo
Copy link
Member Author

praytoo commented Jun 21, 2025

Hi @ryanfkeller @FatCatLikesBeer im running into an issue when I try to run Jekyll to visit the local site the error says I have @import “main” in the file and $absolute_url: “{{ site.url }}”; in the file and that's causing the error but when I initially got the error I deleted it from the file…but like I said the error persists even after I’ve deleted those commands and replaced it with @use "../../_sass/main":, can you help me trouble shoot this issue so I can post the screenshots of the site? Thank you image

@ryanfkeller
Copy link
Member

Hi @praytoo -- you shouldn't need to execute Jekyll manually from the command line like it seems you are doing. Have you read and tried the steps in CONTRIBUTING.md* to serve the website? If so, let me know which step got you stuck.

If not, you'll probably want to revert the non-PR related local changes you made (i.e., restore the file you deleted and undo other changes), then try launching the Jekyll server with docker. The server should dynamically detect code changes and update your locally hosted the website as you go.

*Note that you may need to use docker compose up, rather than docker-compose up.

@praytoo
Copy link
Member Author

praytoo commented Jun 21, 2025

Okay I'll give this a go once I'm back at my laptop and I'll update you as I go thank you @ryanfkeller

@praytoo
Copy link
Member Author

praytoo commented Jun 22, 2025

Hi @ryanfkeller — I’ve been trying to follow the Docker workflow to serve the site locally, but I keep running into persistent gem and bundler errors inside the container.

I’ve tried:

  • docker compose down -v and build --no-cache
  • Deleting and regenerating Gemfile.lock
  • Installing gems (rake, bundler 2.6.9) manually inside the container
  • Running bundle install directly in /srv/jekyll

But I'm still getting errors like Bundler::GemNotFound: Could not find gem 'jekyll (~> 4.4.0)'.

Would love guidance on next steps to get the site running with Docker so I can provide the before/after screenshots.

Thank you

@santiseccovidal santiseccovidal requested a review from nubilaxl June 22, 2025 17:24
@ryanfkeller
Copy link
Member

Hi @praytoo,

Sorry for the trouble you're having! I think you're headed down the right path with docker compose down -v and build --no-cache.

The Bundler::GemNotFound: Could not find gem 'jekyll (~> 4.4.0)' error is unusual because the Docker container should handle all dependencies automatically. I see you mentioned removing Gemfile.lock, which was a good call. However, if you ran Jekyll outside the container, it may have generated other Jekyll artifacts in your local filesystem (like the _site/ build directory, .jekyll-metadata cache file, or potentially .jekyll-cache/) that can still conflict with the container's setup.

When you run docker compose up, these remaining local files get mounted into the container and can override the working setup. Since these files live in your local directory, they persist through docker compose down -v and build --no-cache - those commands can't fix files on your host filesystem.

If this seems possible, my recommendation would be to:

  1. Make sure all your local changes are pushed up to your fork
  2. Create a fresh clone of your website repository
  3. Re-try serving the website from that new clone via docker compose up

This would preserve your actual work while eliminating any local Jekyll artifacts, and hopefully should resolve the gem dependency issue since the container will start in a clean environment.

Let us know how this goes, and please reach out here or on slack if you run into issues, or if this doesn't work! Local environment setup is all part of the fun...

@praytoo
Copy link
Member Author

praytoo commented Jun 24, 2025

hello @FatCatLikesBeer ive updated my original post to reflect the requested before and after screenshots!

@caz002 caz002 removed their request for review June 25, 2025 02:29
Copy link
Member

@FatCatLikesBeer FatCatLikesBeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praytoo The requested changes have been made, PR is approved. Thank you for your hard work, this one seamed like a real headache. I commend you, the effort you put really shows!

@ryanfkeller Thank you so much for the support, your contributions were a total lifeline! Thank you!

@praytoo
Copy link
Member Author

praytoo commented Jun 25, 2025

Thank you @FatCatLikesBeer it was my first ever issue, glad we made it through, the journey was worth it!

@praytoo
Copy link
Member Author

praytoo commented Jun 25, 2025

@ryanfkeller also thank you for your contributions!

@nubilaxl
Copy link
Member

Review ETA: 6 PM 6/26/25
Availability: 10-6 PM Wednesday, Thursday

Copy link
Member

@nubilaxl nubilaxl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! I love what you have done, Praytoo!

@github-project-automation github-project-automation bot moved this from PRs being reviewed to PRs ✅ waiting for merge team in P: HfLA Website: Project Board Jun 26, 2025
@praytoo
Copy link
Member Author

praytoo commented Jun 26, 2025

Thank you @nubilaxl ! It was my first issue ever so I'm happy you liked it!

@mugdhchauhan
Copy link
Member

Nice one! Thanks for working on this. I have verified the changes are accurate and will now merge. 🎉

@mugdhchauhan mugdhchauhan merged commit 3677d90 into hackforla:gh-pages Jun 27, 2025
11 checks passed
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

Development

Successfully merging this pull request may close these issues.

Update Project Profile: Website- Remove Kristen Cardon

5 participants