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

Make our nightly CI checks pass #2356

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jessebraham
Copy link
Member

@jessebraham jessebraham commented Oct 16, 2024

These have been failing for awhile now, thanks to #2351 this became much easier to resolve.

Largely the failures were just due to lifetime elision, some small doc-comment related lint errors as well.

I have allowed the static_mut_refs package-wide for some packages; we are aware of the use of statics, I don't intend to rewrite these for this PR. Can be addressed at some other time.

No changes here warrant a CHANGELOG.md entry IMO.

Successful run: https://github.com/jessebraham/esp-hal/actions/runs/11365052005

@jessebraham jessebraham added the skip-changelog No changelog modification needed label Oct 16, 2024
Copy link
Contributor

@bugadani bugadani left a comment

Choose a reason for hiding this comment

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

I would be somewhat happier if we could fix the lifetime stuff separately after the refactors, but I don't mind rebasing PRs, so we can also just get it over with. Thanks!

@jessebraham
Copy link
Member Author

I would be somewhat happier if we could fix the lifetime stuff separately after the refactors, but I don't mind rebasing PRs, so we can also just get it over with. Thanks!

We can merge in any order, I don't mind having to do some rebasing. This wasn't a ton of work, don't want to make your life too difficult. Just lemme know.

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

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

LGTM!

@jessebraham jessebraham marked this pull request as draft October 21, 2024 07:48
@jessebraham
Copy link
Member Author

Will hold off on merging this to avoid making other contributor's lives difficult for the time being. Will update when it's convenient to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants