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

Protect destructors of some template classes #1655

Merged
merged 1 commit into from May 10, 2019

Conversation

KaustubhShamshery
Copy link
Contributor

@KaustubhShamshery KaustubhShamshery commented May 10, 2019

Description

Zilliqa/Issues/issues/507

Ref Item No. 4 http://www.gotw.ca/publications/mill18.htm

a) you want to allow polymorphic deletion through a base pointer, in which case the destructor must be virtual and public; or b) you don't, in which case the destructor should be nonvirtual and protected, the latter to prevent the unwanted usage.

Backward Compatibility

  • This is not a breaking change
  • This is a breaking change

Review Suggestion

Status

Implementation

  • ready for review

Integration Test (Core Team)

  • local machine test
  • small-scale cloud test

@KaustubhShamshery KaustubhShamshery self-assigned this May 10, 2019
@KaustubhShamshery KaustubhShamshery added the Ready Ready for review label May 10, 2019
@KaustubhShamshery KaustubhShamshery added this to PRs in development in Core via automation May 10, 2019
@ansnunez ansnunez moved this from PRs in development to PRs needing review (please help!) in Core May 10, 2019
@codecov-io
Copy link

codecov-io commented May 10, 2019

Codecov Report

Merging #1655 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #1655   +/-   ##
=======================================
  Coverage   33.53%   33.53%           
=======================================
  Files         269      269           
  Lines       32757    32757           
=======================================
  Hits        10984    10984           
  Misses      21773    21773
Impacted Files Coverage Δ
src/libServer/Server.h 0% <ø> (ø) ⬆️
src/libData/BlockChainData/BlockChain.h 92.3% <100%> (ø) ⬆️

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 474fd5c...afe4840. Read the comment docs.

Core automation moved this from PRs needing review (please help!) to PRs approved - ready to merge! May 10, 2019
@ansnunez ansnunez merged commit 79730ab into master May 10, 2019
Core automation moved this from PRs approved - ready to merge! to PRs done (merged) May 10, 2019
@Gnnng Gnnng deleted the fix/make_protected_destructor branch September 6, 2019 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready Ready for review
Projects
Core
  
PRs done (merged)
Development

Successfully merging this pull request may close these issues.

None yet

4 participants