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

Update documentation #368

Merged
merged 38 commits into from
Mar 27, 2023
Merged

Update documentation #368

merged 38 commits into from
Mar 27, 2023

Conversation

SergioGasquez
Copy link
Member

  • Fix clippy warnings
  • Make CI fail if there are clippy warnings (too_many_arguments lint is allowed)
  • Update docstrings

@jessebraham
Copy link
Member

Sorry, still haven't gotten around to going through all these changes.

Curious though, in the Cargo.toml files there are some formatting changes, and in various source files super imports are converted to crate imports; what was the reason for these changes? They just kind of seem like changing things for the sake of changing them, but I'm not sure if there was some other motivation for this or not.

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Just a small change I'd like to see, plus the question I asked in the previous comment. Overall looks pretty good to me though, thanks for taking care of this!

espflash/src/flasher/mod.rs Outdated Show resolved Hide resolved
@jessebraham
Copy link
Member

Also looks like I've made some changes which have caused conflicts here, so this will need a rebase. Sorry about that, I'll hold off on my other PR fiddling with the Cargo.toml files until this gets merged.

@SergioGasquez
Copy link
Member Author

Curious though, in the Cargo.toml files there are some formatting changes, and in various source files super imports are converted to crate imports; what was the reason for these changes?

The Cargo.toml changes were the result of the formatting applied by VsCode, (I have "format on save" setting). As per the super vs crate, I basically change it in places where we already had an import of crate to unify all the "local" imports. None of those changes add any value, tbh, so I am happy to revert them if you want.

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Thanks for making those requested changes. Regarding the formatting stuff, I'm not too concerned out about it, just was curious. It's fine to just leave it as is, not worth the effort to change it back.

Thanks again for taking care of this!

@jessebraham jessebraham merged commit 3516c4b into esp-rs:main Mar 27, 2023
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.

2 participants