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

Content updates sprint 13 #1715

Merged
merged 16 commits into from
Oct 11, 2022
Merged

Content updates sprint 13 #1715

merged 16 commits into from
Oct 11, 2022

Conversation

bonnieAcameron
Copy link
Contributor

@bonnieAcameron bonnieAcameron commented Jul 14, 2022

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
Copy link
Contributor Author

bonnieAcameron commented Jul 14, 2022

@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

@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
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.

_data/implementations.yml Outdated Show resolved Hide resolved
_data/implementations.yml Outdated Show resolved Hide resolved
_data/implementations.yml Outdated Show resolved Hide resolved
_posts/README.md Show resolved Hide resolved
pages/documentation/migration-V3.md Outdated Show resolved Hide resolved
James' edits implemented
James' edits applied, but clarified without the "above" directive
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

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

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.

Copy link
Member

@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!

pages/documentation/migration-V3.md Outdated Show resolved Hide resolved
pages/documentation/migration-V3.md Outdated Show resolved Hide resolved
@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
Labels
None yet
Projects
None yet
4 participants