Skip to content

Conversation

david-mcafee
Copy link
Contributor

@david-mcafee david-mcafee commented Feb 27, 2023

Description of changes:

While adding TypeScript examples to our DataStore docs, I noticed several existing code examples that were broken. See:
#5156
#5157
#5168
#5231
aws-amplify/amplify-js#11006
aws-amplify/amplify-js#9618

Related docs fixes:
#5155
#5140

Updating our PR template with a check for new / updated E2E tests for all new / updated code examples in the docs.

Related GitHub issue #, if available:

Instructions

If this PR should not be merged upon approval for any reason, please submit as a DRAFT

Which product(s) are affected by this PR (if applicable)?

  • amplify-cli
  • amplify-ui
  • amplify-studio
  • amplify-hosting
  • amplify-libraries

Which platform(s) are affected by this PR (if applicable)?

  • JS
  • iOS
  • Android
  • Flutter
  • React Native

Please add the product(s)/platform(s) affected to the PR title

Checks

  • Does this PR conform to the styleguide?

  • Does this PR include filetypes other than markdown or images? Please add or update unit tests accordingly.

  • Are any files being deleted with this PR? If so, have the needed redirects been created?

  • Are all links in MDX files using the MDX link syntax rather than HTML link syntax?

    ref: MDX: [link](https://link.com)
    HTML: <a href="https://link.com">link</a>

When this PR is ready to merge, please check the box below

  • Ready to merge

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@david-mcafee david-mcafee requested a review from a team as a code owner February 27, 2023 18:21
@david-mcafee david-mcafee changed the title feat(data): update PR template with E2E test check for code examples feat(data): update PR template with E2E test check for code examples; update docs link example to point to an Amplify-owned resource Feb 27, 2023
Comment on lines -34 to -35
_ref: MDX: `[link](https://link.com)`
HTML: `<a href="https://link.com">link</a>`_
Copy link
Contributor Author

Choose a reason for hiding this comment

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

link.com is an actual website, but not owned by Amplify.

@david-mcafee david-mcafee requested review from a team February 27, 2023 18:29
stocaaro
stocaaro previously approved these changes Feb 27, 2023
jimblanc
jimblanc previously approved these changes Feb 27, 2023
@david-mcafee david-mcafee dismissed stale reviews from jimblanc and stocaaro via d66b60a February 27, 2023 22:28
Copy link
Contributor

@nearestnabors nearestnabors left a comment

Choose a reason for hiding this comment

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

I have questions... How are non-Amplify team members expected to participate in this? Is there a way to permanently associate a test with a code snippet? How does creating E2E tests for code snippets impact our writing guidelines?

@timngyn timngyn added the amplify/docs-engineering Issues with the docs site itself, NOT its content label Feb 28, 2023
@david-mcafee
Copy link
Contributor Author

david-mcafee commented Mar 1, 2023

I have questions... How are non-Amplify team members expected to participate in this? Is there a way to permanently associate a test with a code snippet? How does creating E2E tests for code snippets impact our writing guidelines?

@rachelnabors - when you say non-Amplify team members, do you mean external contributors? If so, we would not expect them to write E2E tests. However, we would expect an Amplify team member to add / update the E2E tests as necessary. The reason being - as of today, we have several broken code snippets in our docs, and no existing mechanism to know that they are broken. If the examples live in our testing pipeline, we will be notified if a library change breaks existing docs. Additionally, if we add new examples, tests will allow us to properly catch syntax or business logic errors.

Is there a way to permanently associate a test with a code snippet? - we don't currently have a mechanism for this that I'm aware of, at least with regards to the JS library, though it's something we can certainly explore.

How does creating E2E tests for code snippets impact our writing guidelines? - if you are referring to the styleguide, I can update that, as well.

Let me know if you have further questions!

@nearestnabors
Copy link
Contributor

I see. I recommend that, rather than adding another tick box that doesn't mean anything to external contributors, we add mechanisms to ensure each line of code added has an associated test, like we use to ensure PRs that remove files have redirects. That would be a good part of a project we're working on. I'll put you in touch with the right folks on the effort.

@hbuchel
Copy link
Contributor

hbuchel commented Jan 3, 2024

Pulled out the fix for updating the link.com example link into this separate PR.

Closing this PR due to its age and the docs site has since been refactored. Please feel free to update with latest and re-open if you feel it is still relevant, thanks.

@hbuchel hbuchel closed this Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
amplify/docs-engineering Issues with the docs site itself, NOT its content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants