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 embassy dependencies #19

Merged
merged 12 commits into from
Oct 27, 2024

Conversation

petekubiak
Copy link
Contributor

@petekubiak petekubiak commented Oct 17, 2024

This PR updates the embassy dependencies to the latest version, so that ector can be used with the latest version of embassy (without these changes there is a dependency clash with embassy v0.6.0).
We have also removed the #[feature] flags which have now been made stable, meaning that the repo as a whole no longer requires the nightly toolchain.

An integration test which had broken dependencies has been removed (see discussion below).

@lulf
Copy link
Member

lulf commented Oct 18, 2024

That's a good question, I've cloned your PR and tried to fix it, but I'm not sure what could be the problem here. This must be some kind of cargo resolving issue, because cargo tree --format '{p} {f}' shows that the features on embassy-time for enabling the builtin timer is set.

TBH I don't think the integration tests add any value anymore now that the examples are much easier to run than they used to be.

@jamessizeland
Copy link
Contributor

happy for me to remove the integration test?

@lulf
Copy link
Member

lulf commented Oct 18, 2024

I think it's ok to remove, they don't add much value and has been a source of annoyance for a long time.

@jamessizeland
Copy link
Contributor

I might see if it can run in tokio instead first

@jamessizeland
Copy link
Contributor

no idea! I'll remove and update the PR.

@petekubiak
Copy link
Contributor Author

There's a clippy issue which needs resolving too, it's not in a file we've changed but clippy issues are set as errors. I noticed earlier there's another PR from February which was also aiming to update dependencies, and contains what I now realise is the fix for the clippy issue. Here is the relevant fix:
23f79bf#diff-0e3fec3ae933f1e8228e1156fc37bb667d6c41ed64ed562558d009fccc9860f0

@jamessizeland
Copy link
Contributor

Cool, I'll leave it for you to look at when you're back @petekubiak. We can consolidate the bits from the other PRs too I guess.

@petekubiak petekubiak marked this pull request as ready for review October 24, 2024 12:39
@petekubiak
Copy link
Contributor Author

petekubiak commented Oct 24, 2024

Ok I think this is ready for full review now.

@lulf lulf merged commit 6ca4d4f into drogue-iot:main Oct 27, 2024
1 check passed
@petekubiak petekubiak deleted the update-embassy-dependencies branch October 27, 2024 15:32
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.

3 participants