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

Remove unused method parameters #3806

Merged
merged 3 commits into from
Dec 19, 2019

Conversation

stejbac
Copy link
Contributor

@stejbac stejbac commented Dec 19, 2019

Remove unused parameters from assorted methods (as found by the IDE), excluding abstract or default methods, as well as cases where the parameter is currently unused but is probably intended to be used later.

For most other such methods, either suppress the warning (where the parameter looks like it will be needed later) or actually use the parameter where it probably should have been. The latter applies to a couple of forwarded telescoping method parameters and an injected clock parameter added a few months ago to [Signed|AccountAge]Witness.isDateInTolerance.

Exclude abstract or default methods, as well as cases where the
parameter is currently unused but is probably intended to be used later.
Use 'clock.millis()' instead of "new Date().getTime()" in SignedWitness
& AccountAgeWitness, as the latter may have been left as an oversight.

Also tidy the date field of the toString() methods.
Also fix forwarding of telescoping method parameters in FormBuilder and
FormattingUtils.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Tested a quick trade with signed accounts on Regtest.
@stejbac Just a heads up, that we are planning to not compensate pure refactoring PRs in the future to push the focus more on direct user value. Refactorings are perfectly fine and do have value of course, but we'd like to see them within some other work that has direct user value like in your last PR.

@ripcurlx ripcurlx merged commit b5ddb63 into bisq-network:master Dec 19, 2019
@stejbac
Copy link
Contributor Author

stejbac commented Dec 19, 2019

OK, that's fine. I did have one other non-functional PR prepared (that shouldn't really need testing as it only involves doc comments). After that, I plan to submit some more (non-DAO-related) performance enhancements I've been working on.

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.

2 participants