wallet: Remove unused local variable old_label #14117

Remove unused local variable old_label.

Last use was removed in #13825.

I’m curious to know why these don’t get picked up during review, instead of always “just after” a PR has been merged? If there is a too being used, can it be run on the PRs?

@fanquake I do quite a lot of checking using various automated jobs, this includes: custom linting, running static analysers on the code base, running tests under dynamic analysers, spell checking of documentation, compiling under various compilers with a wide range of warning flags enabled, etc, etc. These jobs currently run only against master due to resource limitations.

Ideally these checks would run in Travis (assuming the false positive rate is near zero and the runtime is low): that would make sure these issues don't reach master, solve the resource limitations and keep the entire process automated. Could it be better? :-)

In order to catch a larger proportion of these issues pre-merge please help reaching that goal by taking a few minutes to review some of these open PRs of mine:

• To allow for automated thread safety checking (-Wthread-safety-analysis) under Travis: please help by reviewing #11634, #11652, #13115, #13123, #13128, #14108 and the tracking issue #13129
• To allow for automated detection of accidentally ignored return values ([[nodiscard]]) under Travis: please help by reviewing #13864 and #13815
• To allow for automated integer overflow/wraparound checking (-fsanitize=integer) under Travis: please help by reviewing #11535 and #11551
• To allow for automated spell checking (codespell) under Travis: please help by reviewing #13954
• To allow for automated checking of Doxygen comments (-Wdocumentation) under Travis: please help by reviewing #13914
• To allow for automated detection of general issues – please help increase signal to noise by fixing/avoiding compiler warnings or static analyzer warnings (by fixing the cause of the warning at hand or by simply guiding the static analyzers better): please help by reviewing #14117 (this PR), #14094, #14088, #13969, #13909, #13897, #13548, #13766, #13546, #13392, #13382 and #13249.
• To improve the linting/fuzzing/testing infrastructure in general: please help by reviewing #14092, #14010 and #14115

Thanks for helping out!

The C++ Core Guidelines sum up my view on the benefits of mechanical checking over human reviewing for this subclass of issues:

Enforcement might be done by code review, by static analysis, by compiler, or by run-time checks. Wherever possible, we prefer “mechanical” checking (humans are slow, inaccurate, and bore easily) and static checking.

I’m curious to know why these don’t get picked up during review, instead of always “just after” a PR has been merged? If there is a too being used, can it be run on the PRs?

IMO, it would make more sense to do a periodical PR that cleans up these things, grouped together, instead of "just after" a PR.

There's really no hurry to get rid of an unused local variable, no need to immediately open a PR.

We've discussed this before.

Agree with @laanwj. If there was a way to make travis yell at you when there is a "unused local variable" or $some_other_non_critical_but_should_be_cleaned_up_before_the_next_major_release_style_issue, then fix them immediately on master if they slip in for whatever reason.

Otherwise a monthly pull request with all minor fixes (or one right before branch-off every six months) should be enough. Note that you can always keep a local master branch with all you fixes, so that your automated jobs are happy during that time.

utCAK 0e5ac7e.

Agree with the batch cleanup.

We could add a GH label specific to "request" @practicalswift analysis so he can filter those?

This can be combined into #14094, as both are removing unused variables.