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

doc(stdlib): doc comments for BaseTrait #1296

Merged
merged 16 commits into from
Jan 20, 2025

Conversation

i582
Copy link
Contributor

@i582 i582 commented Jan 13, 2025

This PR adds more documentation, as an example of the level I would like to see for the entire standard library!

The new documentation for BaseTrait shows how we document code in general.

  • We use /// for documentation comments
  • Any code examples are wrapped in three apostrophes

I think we need an RFC in which we will describe all such rules that everyone in the team agrees with. Everything described above is negotiable and I will be glad to hear your thoughts on how to further change and improve the documentation rules!

Towards #573

@i582 i582 added the scope: stdlib The Tact standard library (src/stdlib) label Jan 13, 2025
@i582 i582 requested a review from a team as a code owner January 13, 2025 11:06
@anton-trunov anton-trunov added this to the v1.6.0 milestone Jan 13, 2025
@anton-trunov
Copy link
Member

what issue is it solving?

@anton-trunov
Copy link
Member

I would argue cells.tact should either be left intact, or split into two files builder.tact (serialization) and slice.tact (deserialization).

@i582
Copy link
Contributor Author

i582 commented Jan 13, 2025

what issue is it solving?

#573
added to description

@i582
Copy link
Contributor Author

i582 commented Jan 13, 2025

I would argue cells.tact should either be left intact, or split into two files builder.tact (serialization) and slice.tact (deserialization).

Yeah, if this file layout is ok, I'll rename this file to cell.tact.

There are now also functions like asSlice for Builder and Slice that should be placed somewhere (together in a file like conversions.tact or split into cell.tact and builder.tact).

@novusnota novusnota self-requested a review January 13, 2025 11:50
@anton-trunov
Copy link
Member

or split into cell.tact and builder.tact

this is exactly what I'm arguing against:

  • it's either everything in one file called cell.tact (or cells.tact as we have now),
  • or builder.tact and slice.tact: you simply cannot do anything with cells as those are opaque (except passing around as stack values)

Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

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

Let's keep the changes for base.tact and revert the rest of them until we have a clear plan

Copy link
Member

@novusnota novusnota left a comment

Choose a reason for hiding this comment

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

Nice job overall, I've only edited descriptions to match the rest of /// comments and their original source — the docs.

stdlib/std/base.tact Outdated Show resolved Hide resolved
stdlib/std/base.tact Outdated Show resolved Hide resolved
stdlib/std/base.tact Outdated Show resolved Hide resolved
stdlib/std/base.tact Outdated Show resolved Hide resolved
stdlib/std/base.tact Outdated Show resolved Hide resolved
stdlib/std/base.tact Outdated Show resolved Hide resolved
@i582 i582 changed the title add better documentation for BaseTrait and Builder add documentation for BaseTrait Jan 20, 2025
@anton-trunov anton-trunov self-assigned this Jan 20, 2025
CHANGELOG.md Outdated Show resolved Hide resolved
src/stdlib/stdlib/std/base.tact Outdated Show resolved Hide resolved
src/stdlib/stdlib/std/base.tact Outdated Show resolved Hide resolved
src/stdlib/stdlib/std/base.tact Outdated Show resolved Hide resolved
@anton-trunov anton-trunov changed the title add documentation for BaseTrait doc(stdlib): doc comments for BaseTrait Jan 20, 2025
Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

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

LGTM, but let's give @novusnota the chance to comment

@anton-trunov anton-trunov merged commit 9c12481 into main Jan 20, 2025
19 of 20 checks passed
@anton-trunov anton-trunov deleted the pmakhnev/stdlib-documentation-builder-base-trait branch January 20, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: stdlib The Tact standard library (src/stdlib)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants