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

docs: add readme to vtl resolvers directory #6718

Merged
merged 5 commits into from Mar 5, 2021
Merged

docs: add readme to vtl resolvers directory #6718

merged 5 commits into from Mar 5, 2021

Conversation

rajrajhans
Copy link
Contributor

Issue #, if available: Closes #6325

Description of changes:
Adds a README to amplify/backend/api/<MyApp>/resolvers directory describing the override process for resolvers. More details in the linked issue.

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

@rajrajhans rajrajhans requested a review from a team as a code owner February 23, 2021 19:20
Copy link
Member

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

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

Hi @rajrajhans thanks for the contribution! I think it would be better to add the README as a separate file in the resources directory of amplify-category-api and then copy the file into the project destination rather than defining the readme as a string. The file could go under a path like amplify-category-api/resources/awscloudformation/resolver-readme/README.md.

Also, it looks like a lint check is failing, so double check that.

cc @renebrandel to comment on the wording of the README

@renebrandel
Copy link
Contributor

Wording looks good to me.

@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #6718 (9aa6410) into master (966726c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6718   +/-   ##
=======================================
  Coverage   56.80%   56.80%           
=======================================
  Files         486      486           
  Lines       22030    22030           
  Branches     4400     4400           
=======================================
  Hits        12515    12515           
  Misses       8638     8638           
  Partials      877      877           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 966726c...9aa6410. Read the comment docs.

@rajrajhans
Copy link
Contributor Author

@edwardfoyle Thank you for the review. I have made the required changes, and fixed the lint issues as well.

…n instead of string literals in fs.copyFileSync
Copy link
Member

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for addressing the comments!

@github-actions
Copy link

github-actions bot commented Mar 6, 2022

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add README to VTL resolvers directory
4 participants