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

feat: Improve usability of manifest types #124

Merged
merged 9 commits into from Dec 5, 2022
Merged

feat: Improve usability of manifest types #124

merged 9 commits into from Dec 5, 2022

Conversation

zshipko
Copy link
Contributor

@zshipko zshipko commented Dec 2, 2022

  • Adds some helper functions for creating a manifest, mostly helpful for the Rust SDK
  • Renames ManifestWasm to Wasm
  • Renames ManifestMemory to MemoryOptions
  • ManifestWasm and ManifestMemory have been marked as deprecated
  • Adds an alias from MemoryOptions::max_pages to MemoryOptions::max in serde specification

@zshipko zshipko changed the title Cleanup manifest Improve usability of manifest types from Rust SDK Dec 2, 2022
@nilslice
Copy link
Member

nilslice commented Dec 2, 2022

nice - just dropping a note to remind us that we will also need to update:
https://extism.org/docs/concepts/manifest

and the graphic that @bhelx generated. ben, what did you use to make that?

@bhelx
Copy link
Contributor

bhelx commented Dec 2, 2022

I believe it was this: https://jlblcc.github.io/json-schema-viewer/

There is probably a way to render it in the docs.

@bhelx
Copy link
Contributor

bhelx commented Dec 2, 2022

Since we've committed to the API, can we do this in a backwards compatible way? or save it for 1.0?

@nilslice
Copy link
Member

nilslice commented Dec 2, 2022

If we're going to make some changes like this, there's another I'd like to consider in the HttpRequest type: header -> headers

@zshipko
Copy link
Contributor Author

zshipko commented Dec 2, 2022

This PR should be backward compatible, with the old names marked as deprecated

@nilslice - I will add headers as an alias for header right now

@zshipko
Copy link
Contributor Author

zshipko commented Dec 2, 2022

I just realized the max/max_pages and header/headers changes weren't present in the json schema because they were only aliases so I swapped the name of the struct fields and the alias names.

@zshipko zshipko added the enhancement New feature or request label Dec 2, 2022
nilslice added a commit to extism/go-pdk that referenced this pull request Dec 5, 2022
@nilslice
Copy link
Member

nilslice commented Dec 5, 2022

Added a commit to include similar name changes throughout the SDKs. Figure we should be consistent. This breaks backward-compatibility though, so we should discuss.

@zshipko zshipko changed the title Improve usability of manifest types from Rust SDK feat: Improve usability of manifest types Dec 5, 2022
Copy link
Member

@nilslice nilslice left a comment

Choose a reason for hiding this comment

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

LGTM if you're ok with it @zshipko

@zshipko zshipko merged commit 9cf54d5 into main Dec 5, 2022
@zshipko zshipko deleted the cleanup-manifest branch December 5, 2022 23:09
zshipko pushed a commit to extism/go-pdk that referenced this pull request Sep 21, 2023
zshipko pushed a commit to extism/go-pdk that referenced this pull request Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants