Conversation
a8b475d to
42bc792
Compare
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl> Closes: #865
42bc792 to
4fdbe0e
Compare
bnoordhuis
left a comment
There was a problem hiding this comment.
+1700 lines is more than I have time to review in-depth at the moment. I'll trust your good judgement.
| Otherwise looks up Symbol.asyncDispose (with Symbol.dispose fallback | ||
| when u8=1) or Symbol.dispose (u8=0) and throws TypeError if missing | ||
| or non-callable. Stack: value -> value, method. */ | ||
| DEF( using_check, 2, 1, 2, u8) |
There was a problem hiding this comment.
AI comments, right? I'd remove them and just let the code do the talking (in general, not just here; I already spot several in quickjs.c)
Rule of thumb: comment why, not what, and only when the why is not obvious when looking at the code itself.
There was a problem hiding this comment.
Yep, and sure thing, I opened as draft do a few more passes on it.
|
Could you please share some good merged PR or a commit with similar stuff added to compare for better understanding ? ❤️ |
|
Not sure what you mean, can you rephrase? This is the implementation itself, there is no other. Ben made an inception PR but it didn't include the new syntax. This PR implements the entire spec. |
I mean not about this exact spec, but another apriori well implemented. for keywords
|
Co-authored-by: Ben Noordhuis info@bnoordhuis.nl
Closes: #865