Add loader reference to the beta docs#9374
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Ok so at first reading, I didn't dare quibble about the formatting (since apart from api-reference we hadn't reworked the other files) but since Sarah asked me to take a look... 😀
Overall it's really just reformatting around Type:. (the surrounding p tag is mainly there to encourage future people who edit the document to place Default: or Since if needed in the same p tag and the empty line is here to prevent a possible bug due to Markdown being embed in HTML tags)
Just be careful from DataStore (sorry the suggestions are not necessarily in order, I forgot to add some comments): I might have made mistakes when changing the types! Maybe I looked at the wrong files or the types exposed to the public are a little different.
On the other hand, if I didn't make any mistakes, is the difference invisible to the user between DataStore and MutableDataStore? If it is visible, maybe these properties should be differentiated in the documentation...
I also left a suggestion regarding DataEntry as it seems that several properties use this type.
Co-authored-by: Luiz Ferraz <luiz@lferraz.com> Co-authored-by: Armand Philippot <git@armand.philippot.eu>
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
|
@ArmandPhilippot Thanks for the detailed review! Addressing the point about the |
Oh sorry! I was sure I had selected the So I have another question: is |
|
I've extracted the |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
A few more quibbles related to formatting and one about an abbreviation.
After this (and my question about addModuleImport), I will have nothing more to say! I'll let others speak. 😄
Thanks @ascorbic for your edits (and your patience with my error 😓 ). Everything looks good to me (including the new DataEntry section)!
Co-authored-by: Armand Philippot <git@armand.philippot.eu>
|
@ArmandPhilippot thanks! And yes, you're right the |
sarah11918
left a comment
There was a problem hiding this comment.
Whew! Great work everyone. I'm so happy with everyone's care and attention to detail in making our newest API page a model for going forward!
I've made a bunch of tiny changes here, like simply breaking up longer paragraphs, that I will commit directly to reduce noise.
In the case of ones where I'd like people to take a look, I'll leave those open and unresolved. But this is super close to being ready to go, and again, thanks for both the initial great content to work with, and the fabulous reviews to make this shine!
sarah11918
left a comment
There was a problem hiding this comment.
Whew! Great work everyone. I'm so happy with everyone's care and attention to detail in making our newest API page a model for going forward!
I've made a bunch of tiny changes here, like simply breaking up longer paragraphs, that I will commit directly to reduce noise.
In the case of ones where I'd like people to take a look, I'll leave those open and unresolved. But this is super close to being ready to go, and again, thanks for both the initial great content to work with, and the fabulous reviews to make this shine!
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
|
@ascorbic I am happy with this PR now! Once you're happy, fix anything I might have messed up etc., it's good to merge! |
Description (required)
Adds a loader API reference page
Related issues & labels (optional)