-
-
Notifications
You must be signed in to change notification settings - Fork 446
Implement Set methods from ECMAScript Specification Features/#4128 #4145
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
Conversation
fix 24.2.4.9 Set.prototype.intersection ( other ) edit documentation from steps fix 24.2.4.5 Set.prototype.difference ( other ) edit documentation from steps
fix 24.2.4.9 Set.prototype.intersection ( other ) edit documentation from steps fix 24.2.4.5 Set.prototype.difference ( other ) edit documentation from steps
done 24.2.4.12 Set.prototype.isSupersetOf ( other )
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4145 +/- ##
==========================================
+ Coverage 47.24% 53.24% +6.00%
==========================================
Files 476 487 +11
Lines 46892 50311 +3419
==========================================
+ Hits 22154 26790 +4636
+ Misses 24738 23521 -1217 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is looking great so far! It looks like you may need to run Also, if you remove cargo run --release --bin boa_tester -- run -vv -s test/built-ins/Set/prototype If you want more verbose feedback, you can run: cargo run --release --bin boa_tester -- run -vvv -s test/built-ins/Set/prototype Adding the specific method on the end of the path can target that specific method. |
i am done all fix need this issus #4128 |
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.
This is looking great so far!
Have some general feedback to help increase test coverage on the implementation and a typo.
…e comment on using GetSetRecord in difference, nit: rename to is_disjoint_from, suggestion: use GetSetRecord abstract op See review comment on difference method. ,
I'm sorry that I've been missing for so long, there were too many blockages at work, so I fixed everything according to your recommendations, and also divided the test logic for some tests. |
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 working on this!!!
This Pull Request fixes/closes #{issue_num}.
It changes the following:
Additional Information:
This implementation aligns with the ECMAScript specification, section 24.2.4, and follows the behavior described in the Set Methods Proposal. Test coverage includes edge cases such as empty sets, overlapping sets, and disjoint sets.
Feel free to let me know if there are areas for improvement or adjustments!