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

Add Go binding #1

Merged
merged 8 commits into from
Sep 1, 2023
Merged

Add Go binding #1

merged 8 commits into from
Sep 1, 2023

Conversation

SteffenL
Copy link
Contributor

@SteffenL SteffenL commented Aug 28, 2023

Part of the work for splitting Go binding from core webview library.

See the following issue:

@ttytm
Copy link

ttytm commented Aug 30, 2023

What about using a script to pull the header files? - stupidly asked, as there might be a good reason you include them.

@SteffenL
Copy link
Contributor Author

SteffenL commented Aug 30, 2023

The headers were copied in because there's no way to pull in external non-Go code such as C/C++ headers with a normal Go workflow. It's a fight against Go tooling that I don't believe we can win.

We've been imposing manual steps or scripts onto Go users. It isn't a nice experience for users. I wrote a Go generator that pulls in headers but it requires users to also manually run the generator. I've tried vendoring but it requires us to sprinkle Go-related files in the repository of the core library which means it isn't much better than leaving the Go bindings there. Oh, and vendoring also requires users to opt into using vendoring (for all of their Go dependencies, not just webview).

This PR accepts the downside of copying headers in order to put an end to the clumsy process we put users through.

@ttytm
Copy link

ttytm commented Aug 30, 2023

Yes security issues make it not justifiable to automatically run a generator.

I think running go generate once after installing the package is a simple enough process to not a be a blocker. But it's neither a deal breaker to copy the headers to the lib. So the reasoning is understandable, thanks for the explanation 👍

@SteffenL SteffenL merged commit 1a304af into master Sep 1, 2023
6 checks passed
@SteffenL SteffenL deleted the add-go-binding branch September 1, 2023 05:41
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.

2 participants