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

feat: create api docs #9

Merged
merged 3 commits into from
Apr 11, 2024
Merged

feat: create api docs #9

merged 3 commits into from
Apr 11, 2024

Conversation

itsacoyote
Copy link
Contributor

What πŸ’»

  • Create API Reference Docs

Why βœ‹

Evidence πŸ“·

Include screenshots, screen recordings, or console output here demonstrating that your changes work as intended

Copy link

github-actions bot commented Apr 1, 2024

Visit the preview URL for this PR (updated for commit 2ba2688):

https://zksync-docs-staging-5eb09--pr9-sf-create-api-docs-jybyg3t4.web.app

(expires Thu, 18 Apr 2024 14:46:51 GMT)

πŸ”₯ via Firebase Hosting GitHub Action 🌎

Sign: bfaafba5fa82d4f63473aaa76a21fabf1fbb3a11

@dutterbutter
Copy link
Contributor

Other feedback, please refer to image:

  1. Looks like a number of endpoints are highlighted in table of contents rather then the first endpoint as I am at the starting of the page
  2. Difficult to see the distinction between endpoints as they seem blocked together. Perhaps adding that above mentioned param description, usage example, and response will provide more division between endpoints.
  3. Capitalize "API"
  4. Once we finalized / documented the endpoints, we should get feedback from Roman on platform on what type of information would be useful / purposeful in an Intro or Overview section. Perhaps there is more info on rate limiting, or something else relevant that we are missing.
    api-reference-feedback

@itsacoyote
Copy link
Contributor Author

@dutterbutter For the feedback about the highlight in the Table of Contents, that's a NuxtUI feature. It's designed so that it highlights all the headers that are visible on the screen as you scroll.

It does not seem like there's a way to modify that at the moment so that only the uppermost header is highlighted.

@itsacoyote itsacoyote force-pushed the sf-create-api-docs branch from fef5786 to 45718af Compare April 2, 2024 16:01
@itsacoyote
Copy link
Contributor Author

@dutterbutter I like the cards design! I did add in some custom icons you could use, like i-zksync-zksync-logo. They're not 100% optimized, but they work in a pinch for now. You can see other custom icons I added in assets/zksync-icons.ts

@dutterbutter dutterbutter marked this pull request as ready for review April 3, 2024 20:56
@dutterbutter dutterbutter requested a review from a team as a code owner April 3, 2024 20:56
@dutterbutter
Copy link
Contributor

@itsacoyote I think this good to be reviewed. The en, snapshot, and net namespaces will likely live in the "Run a node" space but will get the feedback from platform. Other things I will follow up on and update iteratively are the methods that are tagged as TODO. These are not available on mainnet but will be soon.

Copy link
Contributor Author

@itsacoyote itsacoyote 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 the help on the API docs. Got a few comments here and there.

Don't know if this was on your mind, but I know the code blocks can get pretty large since they don't have a scroll feature. I'm looking into a way to set a min-height on them so that we can have scrollable code blocks that don't get too tall.

content/20.api-reference/35.ethereum-rpc.md Outdated Show resolved Hide resolved
content/20.api-reference/35.ethereum-rpc.md Outdated Show resolved Hide resolved
content/20.api-reference/35.ethereum-rpc.md Outdated Show resolved Hide resolved
content/20.api-reference/35.ethereum-rpc.md Show resolved Hide resolved
@itsacoyote itsacoyote marked this pull request as draft April 4, 2024 21:36
@itsacoyote itsacoyote marked this pull request as ready for review April 10, 2024 20:11
@itsacoyote itsacoyote requested a review from dutterbutter April 10, 2024 20:11

1. **DATA, 32 bytes** - hash defining the L2 block.
1. **TracerConfig** - Optional configuration for tracing. Refer to the
[TraceConfig documentation](https://geth.ethereum.org/docs/interacting-with-geth/rpc/ns-debug#traceconfig) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps make this _blank?


Array of objects, each representing a traced call made from the specified block.

:display-partial{path="api-reference/_partials/_trace-object"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason this in tabular format and the other response objects are in a bulleted list?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually like the tabular format as its displayed here but just my own preference.

Copy link
Contributor Author

@itsacoyote itsacoyote Apr 11, 2024

Choose a reason for hiding this comment

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

I'm a little torn on choosing between the two. On one hand, tabular format is good for structuring visually but it is also what can break visually with responsive and smaller screen presentations. Bulleted has less structure which makes it more flexible. I leaned towards emulating what Ethereum uses on its API docs. I also noticed it's easier to "fit" more data in a bulleted list if you want sub-lists under an entry.

Another choice with going with bulleted is that it is much easier to edit versus a markdown table. At some point when I've got time, I'm tempted to just convert this all to a yml format and dynamically generate the page layout. That should make it much easier to adjust the formatting, styling, tabular vs bulleted, etc without compromising the data.

Also to answer your original question, I simply forgot to edit this file and convert from tabular to bulleted.

- **logsBloom**: DATA, 256 Bytes - Bloom filter for light clients to quickly retrieve related logs.
- **type**: QUANTITY - integer of the transaction type, 0x0 for legacy transactions, 0x1 for access list types, 0x2 for dynamic fees.

It also returns either :
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It also returns either :
It also returns either:

Comment on lines 17 to 28
#### Parameters

| **Parameter** | **Type** | **Description** |
|----------------------|-------------|----------------------|
| `subscriptionName` | `String` | Name of the subscription. Valid names include "newHeads" for new block headers, "logs" for new log entries, and others depending on the client's capabilities. |
| `options` | `Object` | Optional filter conditions for the subscription, applicable for subscriptions like "logs" where specific event criteria can be set. |

#### Returns

| **Field** | **Type** | **Description** |
|-----------|----------|------------------------------------------------------|
| `result` | `DATA` | A subscription ID used to identify and manage the subscription. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be consistent with the formatting. Can we change this to reflect the parameter list format being used elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh no, I totally missed this file. 😒

@itsacoyote itsacoyote merged commit 2640353 into staging Apr 11, 2024
7 checks passed
@itsacoyote itsacoyote deleted the sf-create-api-docs branch April 11, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants