search

NGnius / PowerTools

Moved to
https://git.ngni.us/NG-SD-Plugins/PowerTools
GNU General Public License v3.0
414 stars 29 forks source link

Multiplatform Dev #52

Closed pastaq closed 1 year ago

pastaq commented 1 year ago
pastaq commented 1 year ago

This took me way longer than I'm willing to admit. Feedback is appreciated.

pastaq commented 1 year ago

Updated the PR but have a new issue. It works, but I get this warning when compiling:

warning: unused `Result` that must be used
  --> src/main.rs:29:13
   |
29 |             std::fs::copy(&log_filepath, &log_filepath.join(".old"));
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_must_use)]` on by default
   = note: this `Result` may be an `Err` variant, which should be handled

warning: `powertools-rs` (bin "powertools-rs") generated 1 warning

Any advice on solving this error? I was not able to use .expect how I expected as PathBuf doesn't have that method implemented. I think using /tmp for settings and logs in an error state could be useful so I left it as that.

NGnius commented 1 year ago

The copy should have .unwrap() or .expect("error message") chained on it. That one is fine to crash because it's only in debug builds and if that copy fails you've got bigger problems than PowerTools crashing.

Is there anything else left to add to this PR, or is it good to merge?

pastaq commented 1 year ago

All requested changes made. This version runs on my Aya Neo 2021. Ready for merge after review.

pastaq commented 1 year ago

Found an error in release compiling, I should have checked that before. The #[cfg(not(debug_assertions))] on 26 causes a compile issue. Need to change it to let log_filepath = std::path::PathBuf::from(r"/tmp/".to_owned()+&PACKAGE_NAME.to_owned()+".log");