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 article for moralis #951

Merged
merged 30 commits into from
Aug 22, 2024
Merged

Add article for moralis #951

merged 30 commits into from
Aug 22, 2024

Conversation

wuzhong-papermoon
Copy link
Contributor

Description

Adding an article for Moralis

Checklist

  • [ βœ”] I have added a label to this PR 🏷️
  • [ βœ”] I have run my changes through Grammarly
  • [ βœ”] If this requires translations for the moonbeam-docs-cn repo, I have created a ticket for the translations in Jira
  • [ βœ”] If pages have been moved around, I have created an additional PR in moonbeam-mkdocs to update redirects
  • [ βœ”] If pages have been moved around, I have run the move-pages.py script to move the pages and update the image paths on the chinese repo
    • [ βœ”] After the script has been run, I have created an additional PR in moonbeam-docs-cn
  • [ βœ”] If images have been added, I have run the compress-images.py script to compress the images.
  • [ βœ”] If variables (in variables.yml) need to be updated (such as a name change), I have updated the moonbeam-docs-cn repo to use the new variables
  • [ βœ”] If this page requires a disclaimer, I have added one

Corresponding PRs

Please link to any corresponding PRs here.

After Translation Requirements

  • Will need to create PR in moonbeam-docs repo to remove images
  • Will need to create PR in moonbeam-docs repo to remove variables
  • Will need to create PR in moonbeam-mkdocs repo to add redirects for Chinese site
  • No additional PRs are required after the translations are done

Items to be Updated

Please list any of the items that will need to be added or deleted after the translations are done here.

@wuzhong-papermoon wuzhong-papermoon added the A0 - New Content Pull request contains new content pages label Jul 10, 2024
Copy link
Contributor

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

Great starting point! I've added some suggestions for changes to bring this in line with the style guide and help it match our existing similar sections more closely. Many of these suggestions are small punctuation changes, so don't let the number of comments alarm you!

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Show resolved Hide resolved
builders/integrations/indexers/moralis.md Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
@wuzhong-papermoon
Copy link
Contributor Author

Hi @dawnkelly09 Thank you for reviewing the article. Appreciate it!
All the requested changes has been incorporated, would you take a look at them again? Thanks!

Copy link
Contributor

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

My apologies. I misunderstood what the style guide says about colons vs. dashes in lists, which is why I suggested those changes. I've switched them back, which is all that has changed in this review. Thank you for your patience while I am learning.

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
dawnkelly09
dawnkelly09 previously approved these changes Jul 16, 2024
Copy link
Contributor

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

updates applied. lgtm

Copy link
Contributor

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

I'd like to review this before it gets merged, so just marking it as changes requested for right now

Copy link
Contributor

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

Thanks for tackling this!

Some general comments:

  • All of the images should have the browser window in them
  • Use steps when you can to break up the content and provide clear instructions
  • Read through the section headers to make sure they all flow and are consistent
  • Add explanations so the sections aren't just like you can do this and that, without explanation or elaborating on what else can be done or where to find more info

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/.pages Outdated Show resolved Hide resolved
Edit the images
Added missing {target=\_blank} elements for URLs
refactors wording
Copy link
Contributor

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

Getting there!

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
Copy link
Contributor

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

The images still need some work, the browser window should not be orange, it should be a dark color, like the rest of the images on the docs site. I'm sorry, I know this is probably frustrating. I'm going to go through today to make the style guide super explicit on what to do and not to do so that it's more clear moving forward.

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
Copy link
Contributor

@themacexpert themacexpert left a comment

Choose a reason for hiding this comment

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

Looks good! Just found a couple things.

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
Copy link
Contributor

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

I think this looks good! The images look great! Just a couple things though:

  • The arrows in these images aren't the ones that are shown in the style guide. Can you please update it?
  • Can you please also confirm that the size of the circles for the steps are 24x24px?

Copy link
Contributor

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

Followed up with you about the images in slack

@eshaben
Copy link
Contributor

eshaben commented Aug 19, 2024

@themacexpert any remaining comments or all good from your side?

eshaben
eshaben previously approved these changes Aug 21, 2024
Copy link
Contributor

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

themacexpert
themacexpert previously approved these changes Aug 21, 2024
Copy link
Contributor

@themacexpert themacexpert left a comment

Choose a reason for hiding this comment

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

LGTM πŸ‘

builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
builders/integrations/indexers/moralis.md Outdated Show resolved Hide resolved
Copy link
Contributor

@albertov19 albertov19 left a comment

Choose a reason for hiding this comment

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

LGTM

@albertov19 albertov19 merged commit 023acc0 into master Aug 22, 2024
1 check passed
@albertov19 albertov19 deleted the Add_article_moralis branch August 22, 2024 08:46
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.

5 participants