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

Add Queries Section #11

Merged
merged 123 commits into from
Aug 26, 2024
Merged

Add Queries Section #11

merged 123 commits into from
Aug 26, 2024

Conversation

themacexpert
Copy link
Collaborator

@themacexpert themacexpert commented Aug 3, 2024

Description

Adds Wormhole Queries Section.

Checklist

  • Required - I have added a label to this PR 🏷️
  • Required - I have run my changes through Grammarly
  • If pages have been moved, I have created redirects in the wormhole-mkdocs repo

@themacexpert themacexpert requested a review from eshaben August 3, 2024 01:17
@themacexpert themacexpert added the A0 - New Content Pull request contains new content pages label Aug 3, 2024
@eshaben eshaben requested a review from dawnkelly09 August 6, 2024 15:57
Copy link
Collaborator

@dawnkelly09 dawnkelly09 left a comment

Choose a reason for hiding this comment

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

Kevin - thanks for bringing this over. Lots of suggestions for bringing this inline with our standards. Thank you for your continued hard work on this migration!

@eshaben need some input on some capitalization standards for Wormhole. The origin docs are pretty fast and loose with the caps so we'll have some work to do getting that up to standards.

themacexpert and others added 21 commits August 7, 2024 11:18
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
'
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
Co-authored-by: Dawn Kelly <83190195+dawnkelly09@users.noreply.github.com>
@themacexpert themacexpert requested a review from eshaben August 19, 2024 23:49
@themacexpert
Copy link
Collaborator Author

My main issue with the snippets is that it makes it way easier to maintain when we have one file instead of two with the same code. You can add comments or even other lines of code around the selected snippet lines. I've shown how you can do this in my comments

Okay, makes sense. I converted all the relevant ones.

Copy link
Collaborator

@dawnkelly09 dawnkelly09 left a comment

Choose a reason for hiding this comment

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

One question from me on the code examples for Query but other than that, looks great!

Copy link
Collaborator

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

Snippets look good! Thanks for taking care of that!

I responded to the comment about the What Libraries Are Available to Handle Queries? section on the FAQs page, a bulleted list would look nice. Other than that, this LGTM. Also note the merge conflict

@themacexpert themacexpert requested a review from eshaben August 23, 2024 00:39
Copy link
Collaborator

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

Looks like @dawnkelly09 has one remaining comment about the example not working. I did not try it, so I'll leave her to check that and give the final approval

@eshaben eshaben requested a review from dawnkelly09 August 23, 2024 02:04
dawnkelly09
dawnkelly09 previously approved these changes Aug 23, 2024
Copy link
Collaborator

@dawnkelly09 dawnkelly09 left a comment

Choose a reason for hiding this comment

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

I committed some changes surfaced during Vale. This should be in good shape as long as we're happy with how the code is working

@dawnkelly09 dawnkelly09 self-requested a review August 23, 2024 16:53
Copy link
Collaborator

@dawnkelly09 dawnkelly09 left a comment

Choose a reason for hiding this comment

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

Send It 🚀

Copy link
Collaborator

@eshaben eshaben left a comment

Choose a reason for hiding this comment

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

LGTM

@eshaben eshaben merged commit 23aed30 into main Aug 26, 2024
1 check passed
@eshaben eshaben deleted the themacexpert/queries branch August 26, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 - New Content Pull request contains new content pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants