Skip to content

Conversation

bobzhang
Copy link
Contributor

No description provided.

Copy link

Inconsistent placement of error types across modules

Category
Maintainability
Code Snippet
builtin/builtin.mbti lines 44-56 vs json/json.mbti lines 17-30 vs rational/rational.mbti lines 11-15
Recommendation
Ensure all error type definitions are consistently placed in the '// Errors' section. Some files like builtin.mbti moved errors to the Errors section, but the placement within that section varies (some before Types, some after).
Reasoning
Consistency in code organization improves maintainability and makes it easier for developers to locate error definitions across the codebase.

Empty Errors sections added to files without error types

Category
Maintainability
Code Snippet
array/array.mbti line 13, bool/bool.mbti line 6, byte/byte.mbti line 9
Recommendation
Consider only adding the '// Errors' section to files that actually define error types, or establish a clear policy on whether empty sections should be included for consistency.
Reasoning
Empty sections may create unnecessary noise in interface files. Either all files should have all sections for consistency, or sections should only be present when they contain content.

Potential inconsistency in error section ordering

Category
Correctness
Code Snippet
json/json.mbti has Errors section before Types section, while strconv/strconv.mbti has specific error types defined in Errors section
Recommendation
Establish and follow a consistent ordering of sections across all .mbti files. The standard order appears to be: Values, Errors, Types and methods, Type aliases, Traits.
Reasoning
Inconsistent section ordering can cause confusion when developers expect to find information in a specific location across different modules.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 575

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 90.108%

Totals Coverage Status
Change from base Build 574: 0.0%
Covered Lines: 8472
Relevant Lines: 9402

💛 - Coveralls

@bobzhang bobzhang merged commit 0a9c598 into main Jul 29, 2025
11 checks passed
@bobzhang bobzhang deleted the hongbo/fmt_info branch July 29, 2025 07:12
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.

2 participants