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: move extism functions to extism:host/env namespace #31

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

mhmd-azeez
Copy link
Collaborator

This part of the extism/extism/issues/504 effort to move extism functions to extism::env. For backward compatibility reasons, I am also registering the functions in the env namespace.

@mhmd-azeez mhmd-azeez changed the title feat: move extism functions to extism::env namespace feat: move extism functions to extism:env namespace Oct 11, 2023
@nilslice
Copy link
Member

For backward compatibility reasons, I am also registering the functions in the env namespace.

Is this a consistent change we're making across all the SDK options? I'm not certain we should really do this for compatibility alone. The v1.0 rollout gives us a rare opportunity to make incompatible changes, and we should take advantage of that to provide the cleanest implementation and API.

Remember, we will need to support these APIs in v1.X.X for a long time, and if we're duplicating function exposure into multiple namespaces here it sounds like its adding some complexity that could bite us in the future.

@zshipko / @bhelx, would like your thoughts on this too!

@mhmd-azeez
Copy link
Collaborator Author

mhmd-azeez commented Oct 16, 2023

I'm not certain we should really do this for compatibility alone

The reason I added this because I wanted people to still be able to try out Extism while we're rolling out the change across different SDKs/PDKs. We can remove the shims before release 1.0

@nilslice
Copy link
Member

The reason I added this because I wanted people to still be able to try out Extism while we're rolling out the change across different SDKs/PDKs. We can remove the shims before release 1.0

@mhmd-azeez ah that makes sense! ok, let's capture the removal of these on env in a ticket then and make sure we pull them before 1.0. Good thinking -- nice to be able to merge in 1.0 changes while not breaking pre-1.0 code 👍

@zshipko zshipko changed the title feat: move extism functions to extism:env namespace feat: move extism functions to extism:host/env namespace Oct 20, 2023
@zshipko zshipko merged commit 8391ea7 into main Oct 30, 2023
3 checks passed
@zshipko zshipko deleted the feat/rename-namespace branch October 30, 2023 18:44
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.

3 participants