8000 docs: update CONTRIBUTING.md with new guidelines and clarifications by bobzhang · Pull Request #2324 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

docs: update CONTRIBUTING.md with new guidelines and clarifications #2324

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

Merged
merged 1 commit into from
Jun 24, 2025

Conversation

bobzhang
Copy link
Contributor

No description provided.

Copy link
peter-jerry-ye-code-review bot commented Jun 24, 2025
Unclear distinction between inspect and assert_ usage

Category
Correctness
Code Snippet
We encourage you to use inspect over assert in tests, as inspect provides more information about the values being tested and can be updated easilly. For testing in the loop, you may use assert_
Recommendation
Clarify with an example:
'We encourage using inspect over assert in tests for better debugging:

// Instead of: assert(result == expected)
// Use: inspect(result, expected) // Shows actual values on failure

// Exception: Use assert_ in loops where snapshot testing isn't practical
for i in range(0, 10) { assert_(i >= 0) }
```'
**Reasoning**
The current text doesn't clearly explain when to use each assertion type or demonstrate their differences, which could lead to inconsistent testing practices

</details>
<details>

<summary> Typo in documentation </summary>

**Category**
Maintainability
**Code Snippet**
can be updated easilly
**Recommendation**
Fix spelling to 'easily'
**Reasoning**
Correct spelling improves documentation quality and professionalism

</details>
<details>

<summary> Vague guidance on API additions </summary>

**Category**
Maintainability
**Code Snippet**
If the new API can be composed with existing APIs without loosing efficiency
**Recommendation**
Provide concrete criteria: 'If the new API can be composed with existing APIs without performance overhead (i.e., no additional allocations or O(n) complexity increase), prefer composition over new implementations. Example:
```moonbit
// Instead of adding new API: def filter_map(f)
// Use existing: arr.filter(f).map(g)  // If performance is equivalent
```'
**Reasoning**
Clear examples and specific criteria help contributors make better decisions about API additions

</details>

@coveralls
Copy link
Collaborator
coveralls commented Jun 24, 2025

Pull Request Test Coverage Report for Build 7490

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.378%

Totals Coverage Status
Change from base Build 7489: 0.0%
Covered Lines: 3534
Relevant Lines: 3954

💛 - Coveralls

@peter-jerry-ye peter-jerry-ye linked an issue Jun 24, 2025 that may be closed by this pull request
@bobzhang bobzhang force-pushed the hongbo/contributions branch from 42e4c0d to 52906d8 Compare June 24, 2025 08:36
@bobzhang bobzhang enabled auto-merge (rebase) June 24, 2025 08:37
@bobzhang bobzhang merged commit 92fb902 into main Jun 24, 2025
12 checks passed
@bobzhang bobzhang deleted the hongbo/contributions branch June 24, 2025 08:52
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.

Update CONTRIBUTING.md
2 participants
0