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

Code quality #55

Closed
wants to merge 14 commits into from
Closed

Conversation

slowriot
Copy link
Contributor

@slowriot slowriot commented Sep 1, 2021

Description

This PR aims to make miscellaneous code quality improvements through the codebase, in conjunction with more substantive concrete fixes in PRs #53 and #54. Specifically:

  • Disable the DEBUG_LOCKORDER define being set for debug build mode - the two should be unrelated, and the code gated by DEBUG_LOCKORDER is broken in any case
  • Disable warnings on fallthrough, and address all instances where expected fallthrough occurs with the appropriate C++17 attribute - this improves readability and allows catching of future unintended fallthrough bugs
  • Avoid implicit conversions that may alter a value, or make them explicit where that behaviour is desired. Add a placeholder warning to catch more such instances
  • Resolve cases of unnecessary promotions (i.e. double promotion in calculating time interval) and add a warning to catch these in future - performance enhancement
  • Remove unused includes, tidying the code and improving build time slightly
  • Tidy and remove unneeded braces for switch cases that don't require them, improving readability
  • Add compiler flags for additional safety and performance warnings

See commit messages for details of each change.

Note, the content of this PR was originally targeted against master, but the changes have also been added to a separate PR against the janus release branch shortly, to ensure consistency:

BTC/BGL PR reward address

ETH/USDT: 0x50b92AB67A3D3DE8c3506D9287893D9a52655486

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