Skip to content

Conversation

@peterbroadhurst
Copy link
Contributor

For consideration - this seems to reduce the overhead significantly.

Signed-off-by: Peter Broadhurst <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #344 (cec1afc) into main (5ef3cdd) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #344   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          234       234           
  Lines        12793     12793           
=========================================
  Hits         12793     12793           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ef3cdd...cec1afc. Read the comment docs.

@awrichar
Copy link
Contributor

awrichar commented Dec 2, 2021

Guess it's just a cost-benefit discussion. This does remove some checks that have been flagged before (and therefore prompted us to make micro-optimizations). The overall return on these types of optimizations may not be large though.

@peterbroadhurst
Copy link
Contributor Author

Guess it's just a cost-benefit discussion

+1 to this this assessment. Guess I've placed my vote by raising this, and am interested in the other maintainers thoughts.

@nguyer nguyer merged commit 002e6f7 into hyperledger:main Dec 9, 2021
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.

4 participants