Conversation
There was a problem hiding this comment.
@greg6775 Thank you for your contribution!
There is - however - a huge amount of changed files, where the only change is re-formatting - either in the use clauses at the top, or in the file body below where some methods' arguments did become multiline.
Was this strictly necessary - if yes - why and if not why are we doing this?
I think cargo fmt is passing fine and without changes on the current code? If so, can you remove these extra formatting changes as they are really making the actual changes very difficult to spot?
Thanks!
ivmarkov
left a comment
There was a problem hiding this comment.
Two more things:
- Would it be possible not to update
bindgento 0.72?bindgenis part of theembuildpublic API so this means 2 things I'm not sure we should do:- We have to release a new
embuildminor version rather than a patch version - We have to have all esp-idf-* crates refer to the new
embuildminor version
- We have to release a new
Moreover, all dependencies are updated to list explicitly a minor+patchlevel version. I.e. if something was at version "1" now it is at version "1.x.y". This is not strictly necessary unless you really want to depend on that minimum minor version and that minimum patch version in Cargo.toml. But cargo update would anyway raise those in Cargo.lock so what's the need to explicitly list those?
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo fmtcommand to ensure that all changed code is formatted correctly.cargo clippycommand to ensure that all changed code passes latest Clippy nightly lints.CHANGELOG.mdin the proper section.Pull Request Details 📖
Description
Fixes #99