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

Resolve Compiler Warnings #108

Merged
merged 3 commits into from Sep 28, 2023
Merged

Conversation

who-biz
Copy link
Contributor

@who-biz who-biz commented Sep 28, 2023

This, when combined with #106 and #107, resolves all displayed compiler warnings for our given dependency state.

We do have some areas of code that are missing docs. As a result, I commented out many #[warn(missing_docs)] directives in code.

All of these were performed in this commit (line highlighted for demonstration in diff): acc157e#diff-27f4d3db6f3d4186c01baf2c708602ed942e2c994e0aaf61f822c246ef477b5dR22

While not urgent, we should go through and uncomment each of these, and add docs that are missing.

Then, Epic can be published as a crate, for use by CypherStack and others, accompanied by worthy documentation.


Details on warnings these commits resolve:

  1. Update zip dep to 0.5.11, resolve deprecated function warnings 3b58407
warning: use of deprecated method `zip::ZipWriter::<W>::start_file_from_path`: by stripping `..`s from the path, the meaning of paths can change. Use `start_file` instead.
  --> util/src/zip.rs:43:21
   |
43 |             writer.get_mut().start_file_from_path(x, options)?;
   |                              ^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

warning: use of deprecated method `zip::read::ZipFile::<'a>::sanitized_name`: by stripping `..`s from the path, the meaning of paths can change.
                         `mangled_name` can be used if this behaviour is desirable
  --> util/src/zip.rs:63:31
   |
63 |                 let path = dest.join(file.sanitized_name());
   |                                           ^^^^^^^^^^^^^^

warning: use of deprecated method `zip::read::ZipFile::<'a>::sanitized_name`: by stripping `..`s from the path, the meaning of paths can change.
                         `mangled_name` can be used if this behaviour is desirable
   --> util/src/zip.rs:151:24
    |
151 |             let san_name = file.sanitized_name();
    |  

These changes were cross-referenced with Grin upstream, to emulate their choices of replacement functions.


  1. Various warnings resolution 88743b1:

in core/src/core/id.rs:

warning: unused doc comment
  --> core/src/core/id.rs:77:1
   |
77 | / /// We want to sort short_ids in a canonical and consistent manner so we can
78 | | /// verify sort order in the same way we do for full inputs|outputs|kernels
79 | | /// themselves.
   | |_--------------^
   |   |
   |   rustdoc does not generate documentation for macro invocations
   |
   = help: to document an item produced by a macro, the macro must produce the documentation as part of its expansion
   = note: `#[warn(unused_doc_comments)]` on by default

in chain/src/store.rs:

warning: value assigned to `prev_header` is never read
   --> chain/src/store.rs:516:14
    |
516 |                     let mut prev_header = None;
    |                             ^^^^^^^^^^^
    |
    = help: maybe it is overwritten before being read?
    = note: `#[warn(unused_assignments)]` on by default

warning: value assigned to `prev_header` is never read
   --> chain/src/store.rs:759:14
    |
759 |                     let mut prev_header = None;
    |                             ^^^^^^^^^^^
    |
    = help: maybe it is overwritten before being read?

These were resolved by removing duplicate conditions that set prev_header = None. None is now default value, so we only populate it on successful if evaluation. else conditions are irrelevant here.

in src/bin/tui/mining.rs:

warning: variant `PowDifficulty` is never constructed
  --> src/bin/tui/mining.rs:44:2
   |
40 | enum StratumWorkerColumn {
   |      ------------------- variant in this enum
...
44 |     PowDifficulty,
   |     ^^^^^^^^^^^^^
   |
   = note: `StratumWorkerColumn` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` on by default

warning: variant `SecondaryScaling` is never constructed
   --> src/bin/tui/mining.rs:112:2
    |
107 | enum DiffColumn {
    |      ---------- variant in this enum
...
112 |     SecondaryScaling,
    |     ^^^^^^^^^^^^^^^^
    |

  1. Resolve deprecation warnings for DateTime, TimeZone functions 4179d5b

There were a lot of these warnings to resolve, but they boil down to 2 separate ones, really. Examples provided below, but shown once for brevity.

warning: use of deprecated associated function `chrono::DateTime::<Tz>::from_utc`: Use TimeZone::from_utc_datetime() or DateTime::from_naive_utc_and_offset instead
   --> src/bin/tui/mining.rs:134:43
    |
134 |         let datetime: DateTime<Utc> = DateTime::from_utc(naive_datetime, Utc);
    |                                                 ^^^^^^^^

warning: use of deprecated method `chrono::DateTime::<Tz>::timestamp_nanos`: use `timestamp_nanos_opt()` instead
  --> src/bin/tui/status.rs:66:35
   |
66 |                     let start = prev_update_time.timestamp_nanos();
   |   

- These features have been deprecated since 0.5.7 of zip package
- Changes pulled from mimblewimble/grin@1b8acee
core/src/core/id.rs:
- We have documentation annotated macros that do Rust does not generate docs for

chain/src/store.rs:
- Resolves unused assignment warnings
  - These are resolved by removing no-operation conditions within 'else' branches.
  - Instead, we initialize 'prev_header' with 'None', and only populate on successful bool = true

src/bin/tui/mining.rs:
- Comment out variants in Mining tables that are never constructed
- Looks like they were removed due to size constraints of TUI, Rust doesn't like the variants being present when not used
- DateTime implementations are no longer the proper way to do much to the time operations
- Instead, we use TimeZone::from_utc_datetime, which has same return type... Just semantic changes mostly
@who-biz who-biz changed the title Resolve warnings Resolve Compiler Warnings Sep 28, 2023
@who-biz who-biz merged commit da04eeb into EpicCash:master Sep 28, 2023
1 check passed
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.

None yet

4 participants