Updating the Rust versions #5128
Conversation
bendk
force-pushed
the
rust-version
branch
3 times, most recently
from
0e9d43b
to
5d639cf
Compare
|
See also #5114 :) |
bendk
force-pushed
the
rust-version
branch
8 times, most recently
from
e9275ac
to
f27bac0
Compare
|
@mhammond Thanks for pointing that out, I completely missed it. I just released a new UniFFI version today so I think we can bump both the major and minor Rust versions. I also copied some code from Mark's branch. I think this one might be ready to merge the main differences between this and update-rust are:
|
There was a problem hiding this comment.
Looks good to me, but i'd leave it to @mhammond to verify this makes sense given his other PR!
| pub(crate) struct ImportedModule<'a> { | ||
| #[allow(dead_code)] | ||
| pub(crate) id: ModuleId, |
There was a problem hiding this comment.
hmmm @jhugman would it be okay to get rid of this?
| let metrics = self.db.lock().import_multiple(logins, &encdec)?; | ||
| Ok(serde_json::to_string(&metrics)?) | ||
| self.db.lock().import_multiple(logins, &encdec)?; | ||
| Ok(serde_json::to_string(&())?) |
There was a problem hiding this comment.
should we add a comment here, it looks a little unconventional
There was a problem hiding this comment.
- I kept the signature of the
logins::import_multiple()function the same. I think Kotlin is still expecting a string return and didn't want to make a change there.
Yeah - I misread the kotlin code here - I'm sure that result is still being dropped on the floor though, so we should consider getting an issue on file to kill this - maybe a good-first.
- I chose to add more Eq derives rather than adding clippy allows. I don't have a strong feeling about this either way though.
Yeah, I just got sick of it :)
- I added an
[allow(dead_code)]tocomponents/support/nimbus-fml/src/intermediate_representation.rsrather than removing the module_id field. I'm not sure what's best here, I just didn't feel comfortable changing the nimbus code.
Like the Kotlin issue above, I was just trying to be aggressive because I think it's safe to assume that field was a mistake - if it was intentional I'd expect a comment. But yeah, totally fine with this and hopefully Nimbus treats that dead_code annotation as a smell.
This matches the nightly versions for FF desktop.
|
@bendk Should this be added to the changelog? |
|
Good catch, it probably should. #5149 |
This updates it to the min version used by Desktop nightly. I'm not sure if we should track nightly or release in our rust version, but I'm making this PR because I'm hoping it the coverage CI failures. Those are failing with:
I wanted to update the max version to 1.63, but that leads to clippy errors because mozilla/uniffi-rs#1331 has not been released yet.
Pull Request checklist
[ci full]to the PR title.Branch builds: add
[ac: android-components-branch-name]and/or[fenix: fenix-branch-name]to the PR title.