style: fix and enforce clippy linting#385
Conversation
| let _ = crate::release::package_upgrade(|event| { | ||
| let _ = dbus_tx.send(SignalEvent::Upgrade(event)); | ||
| }); | ||
| }).await; |
There was a problem hiding this comment.
this was a bug- without awaiting the future, package_upgrade never ran
| } | ||
|
|
||
| let _ = child.wait(); | ||
| let _ = child.wait().await; |
There was a problem hiding this comment.
another bug- this future never runs in the current implementation
| @@ -1,4 +1 @@ | |||
| fn main() { | |||
| pkg_config::Config::new().probe("pop_system_updater_gtk").unwrap(); | |||
There was a problem hiding this comment.
The pkg_config::probe("pop_system_updater_gtk") call has been non-functional since the project was renamed from pop-system-updater to pop-upgrade- the .pc file is generated as pop_upgrade_gtk.pc but the probe still referenced the old name. It only appeared to work locally because cargo caches build script output and cargo:rerun-if-changed=build.rs prevented re-execution. In CI (no cache), the probe fails and blocks cargo clippy.
The probe is also unnecessary: it links the gtk rlib against the cdylib that gtk/ffi produces from it, which is circular. Actual system library linking (GTK, glib, dbus) is handled by the Rust -sys crate build scripts. Removing the probe and its pkg-config build-dependency fixes CI and has no effect on the workspace build (verified locally).
|
@mmstick does it make sense to bundle the formatting changes with this PR? |
|
That is fine as long as it doesn't modify GitHub's workflow mechanism. |
I'm not sure I understand what you mean here? |
2 participants
adds clippy linting.
marked as a draft since this is stacked on #383 and #384 and these should be merged first
Note on Clippy version
this is currently using the pinned toolchain (
rust-toolchain.toml) as the clippy version. Pinning clippy to a static version prevents new lints causing breakages in PRs which are unrelated to the changes in those PRs. That's the behaviour we want- but it's not strictly required to pin to this version- it could be a newer version of clippy (since clippy respects therust-versionMSRV).i've elected not to do that here, because if you pinned clippy to a recent nightly version, you'd need an automated way to regularly bump that pinned version. Renovate can do that- but this project isn't using Renovate.
Something to consider in future.