pchickey opened PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. If we want to avoid this, we could use cargo features to make the metadata module opt-in. The witx dependency for wasi-common could be hidden behind that feature flag. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for guidance on what the right way to approach this is.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey edited PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton submitted PR Review.
pchickey closed without merge PR #1387.
pchickey reopened PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey updated PR #1387 from pch/wiggle_add_document
to master
:
Over in Lucet, we'll be using some wiggle-adjacent proc macros to generate bindings to wasi-common. I'd like to make the witx document that wiggle used available in wasi-common's public interface so that I can generate the bindings straight from there, eliminating the possibility of getting our wires crossed.
The biggest issue I have with this PR is that it means all users of
wiggle
will have to take awitx
runtime dependency that they otherwise may not need. In order to avoid this, wiggle emits the new metadata behind a new feature flagwiggle_metadata
that consumer crates can opt-in to. As a result, we had to add the witx dependency to wasi-common, but its able to be optional if wasmtime doesnt want to use it. I haven't used cargo features as a library author before, or in a proc macro context, so I'd like to ask @alexcrichton for feedback if I did this right.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey requested alexcrichton for a review on PR #1387.
alexcrichton merged PR #1387.
Last updated: Dec 23 2024 at 12:05 UTC