-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add testing styleguide #1241
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements. In contract-dev/testing/styleguide.mdx: a couple of suggestions to align tone and structure—please apply the inline suggestions.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
/review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update to contract-dev/testing/styleguide.mdx; I have a couple of suggestions to align with the style guide. Please apply the inline suggestions.
|
|
||
| Test descriptions must clearly state what behavior is being verified. Use the pattern "should \[action or outcome]" to make test intent immediately obvious. | ||
|
|
||
| ```typescript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Partial snippets lack required “Not runnable” label
Multiple code blocks are focused excerpts that are not runnable as-is (for example, they reference Blockchain, Contract, or a test runner context without setup). Per the style guide §10.2, partial snippets MUST be labeled “Not runnable” immediately above the fenced block to avoid copy/paste hazards. The first occurrence starts at L24; additional unlabeled partial blocks appear at L42, L57, L73, L98, L130, L144, L177, L209, L235, and L276. Labeling each prevents readers from attempting to execute incomplete examples and aligns with the house style.
| ```typescript | |
| Not runnable |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
contract-dev/testing/styleguide.mdx
Outdated
|
|
||
| ### Discovering minimal fees | ||
|
|
||
| While you can calculate required fees using formulas, empirically discovering the minimal amount provides practical validation. The following pattern uses binary search to efficiently find the threshold: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Second‑person pronoun “you” used in body text
The sentence at L233 addresses the reader with “you”, which violates the pronoun guidance (avoid “you/your” and “we/I/our”) in the style guide §5.8. Use an imperative or neutral construction to keep tone professional, inclusive, and translation‑friendly. The rest of the paragraph remains accurate; only the opening clause needs rephrasing.
| While you can calculate required fees using formulas, empirically discovering the minimal amount provides practical validation. The following pattern uses binary search to efficiently find the threshold: | |
| Calculating required fees using formulas is possible; empirically discovering the minimal amount provides practical validation. |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
verytactical
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marketing language. We don't have time to redo this due to time constraints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates to contract-dev/testing/style-guide.mdx: I have a couple of suggestions to keep the introduction direct and the gas section aligned with the neutral tone guidelines, so please apply the inline suggestions.
|
|
||
| import { Aside } from '/snippets/aside.jsx'; | ||
|
|
||
| This guide explains how to write tests for TON smart contracts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Throat‑clearing meta‑introduction
Line 8 opens the document with a meta-description of the page (“This guide explains how to write tests for TON smart contracts.”) instead of starting directly with the actionable content. The extended style guide explicitly discourages throat-clearing introductions and circular openers in favor of direct, task-focused language. Keeping the opening sentence action-oriented improves clarity and reduces boilerplate for readers who already know they are reading documentation.
| This guide explains how to write tests for TON smart contracts. | |
| Write tests for TON smart contracts. |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| Always validate that your gas constants and fee calculations are sufficient for complete transaction execution. See more details in our [gas documentation](/contract-dev/gas). | ||
|
|
||
| ### Discovering minimal fees | ||
|
|
||
| You can calculate required fees using formulas, but empirically discovering the minimal amount provides practical validation. Use binary search to efficiently find the threshold: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Reader/author pronouns in gas section
The gas section uses both second-person reader pronouns (“your gas constants”, “You can calculate…”) and first-person author pronouns (“our [gas documentation]”), which violates the style guide’s rule against personal pronouns in documentation. The style guide prefers impersonal or imperative wording to keep the tone neutral and consistent. Rewriting these sentences to remove “you/your” and “we/our” while preserving meaning will align this section with the documented style rules. See the “don’t get personal” rule in https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L169-L171 for the underlying guideline.
| Always validate that your gas constants and fee calculations are sufficient for complete transaction execution. See more details in our [gas documentation](/contract-dev/gas). | |
| ### Discovering minimal fees | |
| You can calculate required fees using formulas, but empirically discovering the minimal amount provides practical validation. Use binary search to efficiently find the threshold: | |
| Always validate that gas constants and fee calculations are sufficient for complete transaction execution. See more details in the [gas documentation](/contract-dev/gas). | |
| ### Discovering minimal fees | |
| Calculate required fees using formulas, but empirically discovering the minimal amount provides practical validation. Use binary search to efficiently find the threshold: |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
Closes #166