Implement DataView built-in object
#1662
Merged
Conversation
This file contains 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
Nimpruda
force-pushed
the
implement-dataview
branch
from
d2e8fbc
to
483af94
Compare
|
I don't understand why the CI doesn't pass, it does type check on my computer |
|
The rebase probably made it not typecheck. Can you still compile it after the rebase? |
|
Yup! |
|
Then maybe try to rebase again. If that doesn't work then just add the changes that the CI complains about |
|
Github says that your branch is 16 commits behind the main. It's possible that you rebased against the main of your fork instead of the main of the boa repo. |
Nimpruda
force-pushed
the
implement-dataview
branch
from
483af94
to
dd98b10
Compare
Nimpruda
changed the title
DataView object
Test262 conformance changes:
Fixed tests (932):New panics (2): |
|
I've been having a bit of work lately, I'll finish the implementation remove the panics and add the documentation whenever I have time! |
|
Let me give you a tip for the panics, because I struggled with those detach panics for TypedArrays. You probably have some place in your implementation where you |
Nimpruda
force-pushed
the
implement-dataview
branch
from
0fe8246
to
6229721
Compare
|
I only have 10 tests that fail, I couldnt make them pass I can list them here if you want (They are basically all the same but for different types). |
Nimpruda
force-pushed
the
implement-dataview
branch
3 times, most recently
from
9301890
to
00b5eee
Compare
Test262 conformance changes:
Fixed tests (940): |
RageKnify
requested changes
Oct 20, 2021
raskad
requested changes
Oct 20, 2021
|
I must thank you all guys, I've learned a lot implementing this! |
Nimpruda
force-pushed
the
implement-dataview
branch
from
83c95d2
to
b772817
Compare
Razican
added
builtins
Nimpruda
force-pushed
the
implement-dataview
branch
from
b772817
to
f499f15
Compare
raskad
approved these changes
Oct 22, 2021
RageKnify
approved these changes
Oct 23, 2021
raskad
changed the title
DataView objectDataView built-in object
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I implemented a first version of
DataViewit currently passes 61.48% of the tests. There is still work to increase readability. I am learning to use macros to make the setType and getType methods easier to maintain. I'm also currently working on adding the documentation.This Pull Request fixes/closes #1641
It changes the following:
It's my first big PR so I made a ton of mistakes for sure. Please feel free to rant my code it'll definetly help me improve!