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

Added in Closing Span Tag on the About Us Page - Workforce Development Section #6292

Merged

Conversation

evan-ishibashi
Copy link
Member

@evan-ishibashi evan-ishibashi commented Feb 14, 2024

Fixes #5365

What changes did you make?

  • Added in a Closing span tag on the About Us Page in the Workforce Development Section
  • Compared Docker local host to the existing website, and visually the section has not changed.

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

  • This was added in to resolve a linter error

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

  • No visible changes made to page

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 evan-ishibashi-add-closing-span-5365 gh-pages
git pull https://github.com/evan-ishibashi/website.git add-closing-span-5365

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/evan-ishibashi/website/blob/add-closing-span-5365/CONTRIBUTING.md  

@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 Feature: Refactor HTML size: 0.25pt Can be done in 0.5 to 1.5 hours labels Feb 14, 2024
@danvgar danvgar self-requested a review February 14, 2024 03:10
@roslynwythe roslynwythe requested a review from gaylem February 14, 2024 03:16
@danvgar
Copy link
Member

danvgar commented Feb 14, 2024

I can take a look at this, should be able to review before the end of this week (Fri Feb 16). Thank you!

@gaylem
Copy link
Member

gaylem commented Feb 14, 2024

Availability: Weekdays from 9am to 8pm CST
Review ETA: Wednesday, Feb 14th, 8pm CST

Copy link
Member

@gaylem gaylem 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 @evan-ishibashi!

  • Your branch looks great
  • You correctly linked the issue
  • The span tag has been added correctly

The only advice I have is that the docs indicate to note that no changes to the website have been made under "Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)" in lieu of images.

Could you add a a comment like "No visible changes made to website" under the screenshots section?

Thanks for your work on this! :)

Copy link
Member

@jphamtv jphamtv left a comment

Choose a reason for hiding this comment

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

Hey @evan-ishibashi, the pull request is complete with the correct branch, and the issue is properly linked. Your summary of changes is clear, and I've verified that the code change didn't visually alter the page. Nice work executing the code edit cleanly.

In addition to adding the comment requested by @gaylem, could you please remove the extra bullet points that aren't used in your summary of changes section?

After making these adjustments, re-request a review by clicking on the re-request review circular arrow icon next to the reviewer's name. The PR is ready for approval and merging once those minor edits are completed. Thanks!

Copy link
Member

@jphamtv jphamtv left a comment

Choose a reason for hiding this comment

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

@evan-ishibashi thanks for making the updates. PR is approved.

@freaky4wrld freaky4wrld self-requested a review February 17, 2024 05:59
Copy link
Member

@freaky4wrld freaky4wrld left a comment

Choose a reason for hiding this comment

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

@evan-ishibashi thanks for taking up the issue, @gaylem and @jphamtv thanks for suggesting the changes.
The branches look good, linked issue is correct, changes done correctly in order to resolve linter error, docker-testing was successful too.

PR approved....

@freaky4wrld
Copy link
Member

@gaylem re-review the issue again, and then it can proceed to further merging stage!!

Copy link
Member

@gaylem gaylem left a comment

Choose a reason for hiding this comment

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

Excellent work, @evan-ishibashi! Thanks for making the changes. Approved! :)

@freaky4wrld freaky4wrld merged commit 8d2bf33 into hackforla:gh-pages Feb 17, 2024
11 checks passed
@freaky4wrld freaky4wrld removed the request for review from danvgar February 17, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Refactor HTML good first issue Good for newcomers 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor HTML: _includes/about-page/about-card-platform.html - Workforce Development
5 participants