Re: [pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes

Re: [pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes

[Dietmar Maurer]

Some of those changes may make sense, but it is really 
a PITA to review them. Every change can break the code an introduce new
errors (which already happened in the past).

On the other side, It is hard to see any advantages for 
most of those changes.

Re: [pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes

[Dominik Csapak]

On 4/18/21 16:26, Dietmar Maurer wrote:
> Some of those changes may make sense, but it is really
> a PITA to review them. Every change can break the code an introduce new
> errors (which already happened in the past).
> 

any examples for those errors? (just for my curiosity)

can i do anything to make reviewing easier?

> On the other side, It is hard to see any advantages for
> most of those changes.
> 

IMHO, most changes do make the code better and/or more
readable (e.g. nested ifs, complex types, safety annotation,
absurd extreme comparisons), any specific ones you
dislike?

if there are some classes of warnings/errors that do not
make sense, it would probably be good if we introduce some
custom clippy configs in our repos

[pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes

[Dominik Csapak]

rebased on master, fixed new errors/warnings that popped up

Dominik Csapak (14):
  api2/tape: clippy fixes
  tape/changer: clippy fixes
  tape/drive: clippy fixes
  tape/media_*: clippy fixes
  tape/pool_writer: clippy fixes
  backup: clippy fixes
  pxar: clippy fix `or_fun_call`
  bin: clippy fixes
  tape/*: clippy fixes
  tools: clippy fixes
  config/tape_encryption_keys: clippy fixes
  server/worker_task: clippy fix
  bin/proxmox-file-restore: clippy fixes
  bin/proxmox-restore-daemon: clippy fixes

 src/api2/config/tape_backup_job.rs            |  1 +
 src/api2/tape/backup.rs                       |  2 +-
 src/api2/tape/media.rs                        | 42 +++++++++----------
 src/api2/tape/restore.rs                      | 12 +++---
 src/backup/key_derivation.rs                  |  6 +--
 src/backup/manifest.rs                        |  1 +
 src/bin/docgen.rs                             |  2 +-
 src/bin/pmtx.rs                               |  2 +-
 src/bin/proxmox-file-restore.rs               |  4 +-
 .../proxmox_file_restore/block_driver_qemu.rs |  2 +-
 src/bin/proxmox_file_restore/qemu_helper.rs   |  4 +-
 src/bin/proxmox_restore_daemon/auth.rs        |  8 ++--
 src/config/tape_encryption_keys.rs            |  8 ++--
 src/pxar/extract.rs                           |  4 +-
 src/server/worker_task.rs                     |  2 +-
 src/tape/changer/online_status_map.rs         |  4 +-
 src/tape/changer/sg_pt_changer.rs             |  2 +-
 src/tape/drive/lto/mod.rs                     | 30 +++++++------
 src/tape/drive/lto/sg_tape.rs                 | 18 ++++----
 src/tape/drive/lto/sg_tape/mam.rs             | 12 +++---
 src/tape/drive/mod.rs                         |  4 +-
 src/tape/drive/virtual_tape.rs                |  4 +-
 src/tape/file_formats/blocked_reader.rs       |  4 +-
 src/tape/file_formats/chunk_archive.rs        | 12 +++---
 src/tape/file_formats/multi_volume_writer.rs  |  2 +-
 src/tape/inventory.rs                         |  2 +-
 src/tape/media_catalog.rs                     | 10 +++--
 src/tape/media_pool.rs                        | 16 +++----
 src/tape/media_set.rs                         |  4 ++
 src/tape/pool_writer/catalog_set.rs           |  1 +
 src/tape/pool_writer/mod.rs                   |  4 +-
 src/tape/pool_writer/new_chunks_iterator.rs   |  4 +-
 src/tools/cpio.rs                             |  1 +
 src/tools/fs.rs                               |  3 ++
 src/tools/sgutils2.rs                         | 18 ++++----
 35 files changed, 135 insertions(+), 120 deletions(-)

-- 
2.20.1