Skip to content
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

Update to latest JavaScriptKit #8

Merged
merged 24 commits into from Dec 26, 2020

Conversation

j-f1
Copy link
Collaborator

@j-f1 j-f1 commented Sep 15, 2020

Pending the merger of swiftwasm/JavaScriptKit#26, everything needed for the output of webidl2swift to work will be included in the shipping version of JavaScriptKit! I made several changes to align the APIs and fixed a bug where callback protocols could not be passed as a function.

@MaxDesiatov
Copy link
Collaborator

Hi @Unkaputtbar, @PSchmiedmayer, would you be able to have a look at this PR at some point? Otherwise, if you need any help in maintaining the project, please let me know. The SwiftWasm team would be happy to help, as it became quite important for us while developing DOMKit.

Copy link
Collaborator

@mjburghard mjburghard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay. I am pretty busy with other stuff right now.
I had a look at your changes and think they are fine in general.

I will talk to @PSchmiedmayer to see if we can get one of you merge rights, too.
It's cool that you are using the stuff we built.

@PSchmiedmayer
Copy link

Hi @j-f1 @MaxDesiatov,
great to hear that you are using webidl2swift and that it is an important part of the Swift WASM ecosystem!
@Unkaputtbar thanks for taking a look at the PR.
We are happy to accept the help in maintaining the project. I have granted both of you maintenance rights for the repo. 👍

@MaxDesiatov
Copy link
Collaborator

Fantastic, I very much appreciate it!

MaxDesiatov
MaxDesiatov previously approved these changes Oct 1, 2020
Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK if void to undefined change was really needed (maybe needs an explanatory comment at case unknown or some other better suited location?), but other than line length nitpicks this seems legit.

…ode.swift

Co-authored-by: Max Desiatov <max@desiatov.com>
@j-f1
Copy link
Collaborator Author

j-f1 commented Oct 1, 2020

IDK if void to undefined change was really needed

They changed the WebIDL spec from using void to undefined. I’ve added support back for void since a bunch of specs have not been updated yet.

Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MaxDesiatov MaxDesiatov merged commit f8fdafb into Apodini:develop Dec 26, 2020
@j-f1 j-f1 deleted the update-to-latest-jskit branch December 26, 2020 22:10
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.

None yet

4 participants