Skip to content

Content updates sprint 13 #1715

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.

Sign up for GitHub

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

Jump to bottom
Merged
thisisdano merged 16 commits into from
Oct 11, 2022
Merged

Content updates sprint 13 #1715

thisisdano merged 16 commits into from
Oct 11, 2022

Conversation

@bonnieAcameron
Copy link
Contributor

@bonnieAcameron bonnieAcameron commented Jul 14, 2022 •
edited by amyleadem
Loading

Content updates sprint 13

Description

This PR addresses several small but key clarifying points on the site. It will close:

Additional information

Include any of the following (as necessary):

  • For Remove jekyll draft guidance #1584 Remove jekyll guidance, Dan and I reviewed that page together and he decided to delete it altogether.
  • Type of content review needed: stylistic, copy editing, correct pull from implementations.yml to implementations.md

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

@bonnieAcameron bonnieAcameron self-assigned this Jul 14, 2022
This was linked to issues Jul 14, 2022
Documentation: Add ngx-uswds to implementations page #1494
Closed
Remove jekyll draft guidance #1584
Closed
Migration guide links to v2.x theme tokens #1593
Closed
@bonnieAcameron
Copy link
Contributor Author

bonnieAcameron commented Jul 14, 2022 •
edited
Loading

@amyleadem and @mejiaj, I'm not sure why I had so many failures on this. Several of them passed but now show failure again. Could it be because one of the pages is being deleted?

@amyleadem
Copy link
Contributor

amyleadem commented Jul 14, 2022 •
edited
Loading

@amyleadem and @mejiaj, I'm not sure why I had so many failures on this. Several of them passed but now show failure again. Could it be because one of the pages is being deleted?

@bonnieAcameron This one seems to be a two-parter :)

  1. .yml files are space sensitive, so removing the extra space in front of - name should resolve some errors.
  2. We pushed some updates to the main branch this morning to repair some build errors. To get those changes integrated here, you'll need to update the base branch (main) in this PR. However, I am not seeing this option in GitHub (GitHub docs about syncing with base branch). For now, I can update the base branch for you and hopefully that will fix it. Fingers crossed!

Edit: I went ahead and did both updates so that I can confirm that this addresses all the errors.

Another edit: Build was successful! Tomorrow we can circle back and figure out how to update the base branch from the GitHub view. Here's hoping I just have EOD brain and it will be where I expect tomorrow morning :)

mejiaj
mejiaj previously requested changes Jul 15, 2022
View reviewed changes
Copy link
Contributor

@mejiaj mejiaj 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 submitting this PR, Bonnie! I've added some questions and some minor requests related to formatting.

@bonnieAcameron
Update implementations.yml
7a790f8 
James' edits implemented
@bonnieAcameron
Update migration-V3.md
96498b1 
James' edits applied, but clarified without the "above" directive
bonnieAcameron
bonnieAcameron commented Aug 31, 2022
View reviewed changes
Copy link
Contributor Author

@bonnieAcameron bonnieAcameron left a comment

Choose a reason for hiding this comment

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

Applied all of these, thank you James!

@bonnieAcameron
Copy link
Contributor Author

bonnieAcameron commented Aug 31, 2022

@mejiaj and @amyleadem , I'm a bit confused about where this PR stands. What are next steps?

amyleadem
amyleadem reviewed Sep 7, 2022
View reviewed changes
amyleadem
amyleadem previously requested changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

@bonnieAcameron @mejiaj Got the build to work. I had one quick question, but other than that, looks good to me.

thisisdano
thisisdano reviewed Oct 11, 2022
View reviewed changes
Copy link
Contributor

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

A couple very minor edits that I can commit!

@thisisdano thisisdano dismissed stale reviews from amyleadem and mejiaj October 11, 2022 16:33

Resolved issue

@thisisdano thisisdano merged commit 369bae8 into main Oct 11, 2022
@thisisdano thisisdano deleted the content-updates-sprint-13 branch October 11, 2022 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@thisisdano thisisdano thisisdano approved these changes

@mejiaj mejiaj mejiaj left review comments

@amyleadem amyleadem amyleadem left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@bonnieAcameron bonnieAcameron

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migration guide links to v2.x theme tokens Remove jekyll draft guidance Documentation: Add ngx-uswds to implementations page

5 participants

@bonnieAcameron @amyleadem @mejiaj @thisisdano