Record type misleading issue #6426
Conversation
1. Renamed the variable json to data throughout the section. 2. Updated the text to clarify that the example works with deserialized data (e.g., parsed from JSON) rather than raw JSON strings. 3. Ensured consistency in variable naming and explanations. This update makes the documentation more accurate and avoids misleading readers into thinking that the variable holds a JSON string. If you’re contributing to the Dart documentation, you can submit this change as a pull request to the [site-www repository](https://github.com/dart-lang/site-www)
1. **Added ypedef Explanation:** - Added a section explaining how to use ypedef to name record types. - Provided an example of defining and using a UserProfile record type. 2. **Improved Readability:** - Clarified that ypedef can be used to improve code readability and reusability.
|
@parlough can you review this PR please |
|
@parlough just a follow up comment for reviewing this PR |
There was a problem hiding this comment.
Thanks so much for this thoughtful PR @MiniPiku! And sorry again for the delay in reviewing it.
It looks like a lot of this was covered in #6382 which was reviewed over the past month and resulted in a new Records and typedefs section.
Could you take a look at that new section and see if its content is sufficient for what you were imagining? If it is, then perhaps you could simplify your PR to instead cross link to that new section from the "Record types" section to avoid repeating too much.
Let me know what you think. Thanks again :D
There was a problem hiding this comment.
It looks like this PR also has some remnants from other branches. Let me know if you need help removing these unrelated changes.
There was a problem hiding this comment.
can you mention the area that are not related to this PR
will delete those files from commit
There was a problem hiding this comment.
It seems you removed most of those changes now, thanks!
The records.md~ (with a ~ after the filename) file is still there though. It seems to be an accidental copy of the records.md file.
MiniPiku
force-pushed
the
Record_type
branch
from
88beed4 to
20f4071
Compare
There was a problem hiding this comment.
It seems you removed most of those changes now, thanks!
The records.md~ (with a ~ after the filename) file is still there though. It seems to be an accidental copy of the records.md file.
| There is no type declaration for individual record types. Records are structurally | ||
| typed based on the types of their fields. A record's _shape_ (the set of its fields, | ||
| the fields' types, and their names, if any) uniquely determines the type of a record. | ||
| Records are structurally typed based on the types of their fields. |
There was a problem hiding this comment.
It seems my review comment got lost here, perhaps from the force push? I'm not sure, sorry about that!
I think adding this clarification is nice, but from another PR worked on last month, a new section called Records and typedefs was added that already covers much of this.
What do you think about keeping this introduction mostly the same as it was, and adding new text cross-link to that new section? That way this text can be shorter and focused and we don't end up repeating ourselves.
Let me know. I can help brainstorm some cross-link text as well if you get stuck. Thanks again for working on this :)
There was a problem hiding this comment.
yeah i guess the crosslink would be better in this case
should i delete the previous content and just add the crosslink with a single line introduction text
or the previous text remains same with the crosslink addition?
There was a problem hiding this comment.
and how do i resolve the experterts are up-to-date issue
as dart run dart_site refresh-excerpts --fail-on-update --dry-run this cmd is not working
Key Changes:
Added
typedefExplanation:typedefto name record types.UserProfilerecord type.Improved Readability:
typedefcan be used to improve code readability and reusability.