You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix some unused trait implementation. Thanks to simplification of type system, unused warning for trait implementations is under development and will soonish be available. This PR fixes core.
The Eq trait implementation for Entry is moved to a test file which could cause confusion
Category
Maintainability Code Snippet
impl[K : Eq, V] Eq for Entry[K, V] with op_equal(self, other) {
self.hash == other.hash && self.key == other.key
} Recommendation
Consider keeping the Eq implementation in the main source file linked_hash_map.mbt if it's part of the public API. If it's only needed for tests, add a comment explaining this. Reasoning
Moving implementations to test files can make the code harder to maintain and understand, as users would expect to find trait implementations in the main source files. If this implementation is truly only needed for tests, it should be clearly documented.
Tree Show implementation test cases could be more comprehensive
Category
Correctness Code Snippet
test "Show for Tree" {
inspect((Empty : Tree[Int]), content="Empty")
...
} Recommendation
Add test cases for more complex tree structures and edge cases, such as deeply nested trees or trees with mixed node types. Reasoning
While the current test cases cover basic scenarios, tree structures can be complex and the Show implementation should be verified to work correctly for all possible tree configurations.
Removal of #coverage.skip annotation without explanation
Category
Maintainability Code Snippet
/// Print the tree structure. For debug use only.
#coverage.skip
impl[A : Show] Show for Tree[A] with output(self, logger : &Logger) { Recommendation
Add a comment explaining why the coverage skip annotation was removed, or if this implementation should now be covered by tests. Reasoning
Changes to coverage annotations should be documented to help maintain code quality standards and testing requirements. The removal might indicate a change in testing strategy that should be communicated.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix some unused trait implementation. Thanks to simplification of type system, unused warning for trait implementations is under development and will soonish be available. This PR fixes core.