Create the esp-metadata package, update esp-hal build script to use it
#1256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Same for me. There might be slightly more maintenance overhead having it split whenever we need changes in both at the same time - not sure if that will ever happen Another way to think about it might be: Is it likely some crate will use one but doesn't need the other. I think that is true. I can imagine crates using esp-build but don't need esp-metadata This so far already looks quite good to me |
jessebraham
force-pushed
the
feature/esp-metadata
branch
from
39823cf
to
f6b7b99
Compare
|
I've rebased this branch and updated |
JurajSadel
approved these changes
Mar 12, 2024
jessebraham
force-pushed
the
feature/esp-metadata
branch
from
f6b7b99
to
63e811c
Compare
jessebraham
force-pushed
the
feature/esp-metadata
branch
from
63e811c
to
a0bfbd7
Compare
|
I guess this is ready for final review/merging now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I still need to fill out
README.mdand write some proper documentation for this package, so opening as a draft for now, but I the code should be fine so I'd like to get some input on this.We had also discussed creating a build utilities package, so I'm not sure if we want to have
esp-metadataandesp-build(or whatever we name it), or if we want to just try and create a single package. Happy to hear any input, I'm kind of a fan of smaller single-use packages in this particular case but I don't feel strongly about this.If anybody can think of any other functionality that would be useful, I'm more than happy to add it in this PR as well, so just let me know.