a-detiste / cruft-ng

rewrite of Debian "cruft" package
GNU General Public License v2.0
24 stars 5 forks source link

Redundant `if` in dh_cruft #7

Closed nthykier closed 2 years ago

nthykier commented 2 years ago

https://github.com/a-detiste/cruft-ng/blob/1b977cf91598ab5e921f142410ae19eda5ad4bf1/dh-cruft/dh_cruft#L73-L76

The highlighted if appears to be redundant the later if is always run and it includes the body of the redundant if:

https://github.com/a-detiste/cruft-ng/blob/1b977cf91598ab5e921f142410ae19eda5ad4bf1/dh-cruft/dh_cruft#L79-L90

a-detiste commented 2 years ago

I tried to rely on install_dh_config_file() as much as possible (maybe this keeps original timestamp) that's why the logic feels convoluted.

nthykier commented 2 years ago

I tried to rely on install_dh_config_file() as much as possible (maybe this keeps original timestamp) that's why the logic feels convoluted.

I do not feel that answers my remark. The initial if that I feel is redundant is never run except when the larger if is also run. Since the larger if has all of the contents of the smaller one, then smaller one is just dead-weight in my book.

As for preserving the mtime of the file, then you would need to fix the append-case in the larger if (the open/close should bump the mtime unless you reset it manually)

a-detiste commented 2 years ago

Ok, simplicity is always better

a-detiste commented 2 years ago

I feel dumb now :-)