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

docs: Update README #17

Merged
merged 20 commits into from
Nov 24, 2023
Merged

docs: Update README #17

merged 20 commits into from
Nov 24, 2023

Conversation

mhmd-azeez
Copy link
Contributor

@mhmd-azeez mhmd-azeez commented Nov 21, 2023

  • Write a similar README to go-pdk https://github.com/extism/go-pdk
  • Change the API to make it more convenient
    • Move allocate, allocateBytes, findMemory to Plugin. Since Memory is not public, there was no way to call them
    • Added setError and setErrorMemory
    • Added getVarInt and setVarInt
  • fix build errors

Fixes #16

@mhmd-azeez
Copy link
Contributor Author

Not sure why there were some build errors on my machine

@nilslice
Copy link
Member

@mhmd-azeez - thank you! which version of Zig are you using with this?

I'm on 0.12.0-dev.978+78855bd21, and seem to need to use std.builtin.Endian.Little (vs. std.builtin.Endian.little, note the capitalization of little). I am not sure if this is a version thing or a typo haha

@mhmd-azeez
Copy link
Contributor Author

@nilslice seems like they have changed it to little in the newer versions:

PS D:\dylibso\zig-pdk> zig build
zig build-exe Basic example Debug wasm32-freestanding-musl: error: the following command failed with 2 compilation errors:
D:\tools\zig.exe build-exe D:\dylibso\zig-pdk\examples\basic.zig --cache-dir D:\dylibso\zig-pdk\zig-cache --global-cache-dir C:\Users\muham\AppData\Local\zig --name Basic example -rdynamic -target wasm32-freestanding-musl -mcpu generic --mod extism-pdk::D:\dylibso\zig-pdk\src\main.zig --deps extism-pdk --listen=-
Build Summary: 0/3 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install Basic example transitive failure
   └─ zig build-exe Basic example Debug wasm32-freestanding-musl 2 errors
src\main.zig:31:73: error: enum 'builtin.Endian' has no member named 'Little'
            std.mem.writeInt(u64, buf[i..][0..8], x, std.builtin.Endian.Little);
                                                                        ^~~~~~
D:\tools\lib\std\builtin.zig:452:20: note: enum declared here
pub const Endian = enum {
                   ^~~~
referenced by:
    count_vowels: examples\basic.zig:19:25
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
src\Memory.zig:42:78: error: enum 'builtin.Endian' has no member named 'Little'
        const data = std.mem.readInt(u64, buf[i..][0..8], std.builtin.Endian.Little);
                                                                             ^~~~~~
D:\tools\lib\std\builtin.zig:452:20: note: enum declared here
pub const Endian = enum {
                   ^~~~
PS D:\dylibso\zig-pdk> zig version
0.12.0-dev.1664+8ca4a5240

@mhmd-azeez
Copy link
Contributor Author

Hah! it's been changed 3 weeks ago: ziglang/zig@3fc6fc6

@nilslice
Copy link
Member

haha! thank you for looking at that. sorry I was behind a few versions... I think our strategy has been to follow the latest versions as close as possible.

@usdogu can advise, as the expert Zig extism contributor!

@mhmd-azeez mhmd-azeez marked this pull request as ready for review November 22, 2023 12:21
@mhmd-azeez
Copy link
Contributor Author

In addition to reviewing the API changes, please also take a close look at the examples to make sure allocations and deallocations are properly managed

@bhelx
Copy link
Contributor

bhelx commented Nov 22, 2023

Thanks for taking this one on!

Copy link
Collaborator

@usdogu usdogu left a comment

Choose a reason for hiding this comment

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

Other than these, LGTM!

src/main.zig Outdated Show resolved Hide resolved
src/main.zig Outdated Show resolved Hide resolved
src/main.zig Outdated Show resolved Hide resolved
src/main.zig Outdated Show resolved Hide resolved
Co-authored-by: Doğu Us <73913444+usdogu@users.noreply.github.com>
@mhmd-azeez
Copy link
Contributor Author

@usdogu thank you very much for you suggestions!

@mhmd-azeez
Copy link
Contributor Author

mhmd-azeez commented Nov 23, 2023

Also added the readme examples to basic.zig, we can turn them into integration tests

@nilslice
Copy link
Member

Going to merge this one & make sure #19 works based on it

@nilslice nilslice merged commit 8f4ce76 into main Nov 24, 2023
2 checks passed
@nilslice nilslice deleted the update-readme branch November 24, 2023 17:57
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.

Zig PDK README
4 participants