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

commented out broken site links #5422

Conversation

JanineSooThow
Copy link
Member

Fixes #4923

What changes did you make?

  • Commented out site link under file work-for-la.md
  • Commented out site link under file hellogov.md
  • Commented out site link under file jobs-for-hope.md

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

  • The site links for work-for-la.md, hellogov.md and jobs-for-hope.md were not working.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

image
workforla
hellogov
jobs-for-hope

Visuals after changes are applied

image
after-hellogov
after-jobsforhope
after-workforla

@github-actions
Copy link

github-actions bot commented Sep 3, 2023

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 JanineSooThow-4923-comment-out-broken-site-links-for-workforla-hellogov-jobs-for-hope gh-pages
git pull https://github.com/JanineSooThow/website.git 4923-comment-out-broken-site-links-for-workforla-hellogov-jobs-for-hope

@github-actions github-actions bot added role: front end Tasks for front 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 Complexity: Small Take this type of issues after the successful merge of your second good first issue P-Feature: Projects page https://www.hackforla.org/projects/ size: 0.25pt Can be done in 0.5 to 1.5 hours labels Sep 3, 2023
@brianf4 brianf4 self-assigned this Sep 3, 2023
@ortegaa32 ortegaa32 self-requested a review September 3, 2023 22:51
@ortegaa32
Copy link
Member

Review ETA: 3 PM 9/5/23
Availability: 1-3 PM everyday

@ronaldpaek ronaldpaek self-requested a review September 4, 2023 13:22
@ronaldpaek
Copy link
Member

I plan to review this by Sept 7th.

@brianf4 brianf4 removed their assignment Sep 6, 2023
@ortegaa32
Copy link
Member

  • Pull request contains correct branch and links to correct issue
  • Changes accurately reflected in screenshots and don't appear to break the website
  • Changes made to the right files and source code looks clean.

Well done @JanineSooThow. Approving you pull request.

ortegaa32
ortegaa32 previously approved these changes Sep 6, 2023
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 @JanineSooThow You have the correct from-to branches, you are linked to the original issue, you are explaining what you did and why, and you are providing before and after screenshots. Great job!

I have thing that I would like to request that was not specified on the original issue- you followed the issue correctly. This also does not affect the visuals on the website but instead involves the code itself: when commenting out the - name: and url: the code will appear cleaner and easier to read if the hashtags are aligned like so:

  # - name: Site
  #   url: 'https://hellogov.squarespace.com'
  # - name: Site
  #   url: 'https://jobs-for-hope.herokuapp.com/'
  # - name: Site
  #   url: 'http://www.workfor.la/'

Again, you did the issue correctly! If you would make this change, I think the code will appear cleaner.

Thank you for working on this issue!

@JanineSooThow
Copy link
Member Author

Hello @ortegaa32 thank you so much for reviewing my changes :D

@JanineSooThow
Copy link
Member Author

Hello @t-will-gillis thank you so much for the feedback! I really appreciate it! I made the changes on the branch '4923-comment-out-broken-site-links-for-workforla-hellogov-jobs-for-hope'. The changes definitely make the code look cleaner :D. I am not sure if I did the PR correctly , so please let me know if you can see my changes or not.

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 @JanineSooThow Thanks so much for making the changes- Everything looks perfect!

@t-will-gillis t-will-gillis merged commit 6799e3e into hackforla:gh-pages Sep 11, 2023
3 checks passed
sheehanrobb pushed a commit to sheehanrobb/website that referenced this pull request Sep 14, 2023
* commented out broken site links

* aligned hastags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Small Take this type of issues after the successful merge of your second good first issue P-Feature: Project Info and Page A project's detail page (e.g. https://www.hackforla.org/projects/100-automations) P-Feature: Projects page https://www.hackforla.org/projects/ 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
None yet
Development

Successfully merging this pull request may close these issues.

Comment out broken site links for WorkForLA, HelloGOV, Jobs for Hope
5 participants