Skip to content

Remove unused code #9987

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 18, 2017
Merged

Remove unused code #9987

merged 1 commit into from
Mar 18, 2017

Conversation

practicalswift
Copy link
Contributor

No description provided.

dcousens
dcousens approved these changes Mar 14, 2017
View reviewed changes
Copy link
Contributor

@dcousens dcousens left a comment

Choose a reason for hiding this comment

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

utACK

@NicolasDorier
Copy link
Contributor

utACK

paveljanik
paveljanik reviewed Mar 14, 2017
View reviewed changes
src/dbwrapper.h
@@ -132,10 +132,6 @@ class CDBIterator
return true;
}

unsigned int GetKeySize() {
Copy link
Contributor

Choose a reason for hiding this comment

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

GetKeySize is referenced in the comment in src/coins.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paveljanik Good point! Reference in src/coins.h removed. Squashed and pushed as well :-)

@paveljanik paveljanik mentioned this pull request Mar 15, 2017
Closed
@JeremyRubin
Copy link
Contributor

I'd suggest reverting interpreter.cpp, unless anyone strongly objects. Seeing those (albeit unused) static consts helped me to understand the script code more quickly.

@practicalswift
Copy link
Contributor Author

@JeremyRubin Perhaps I should add them back but commented out. Sounds good? :-)

@laanwj
Copy link
Member

laanwj commented Mar 16, 2017

I'd suggest reverting interpreter.cpp, unless anyone strongly objects. Seeing those (albeit unused) static consts helped me to understand the script code more quickly.

Agree, let's just put those back. There is zero overhead to defining a few extra constants for clarity even if unused.

laanwj
laanwj reviewed Mar 16, 2017
View reviewed changes
src/wallet/db.h Outdated
@@ -300,12 +300,6 @@ class CDB
return (ret == 0);
}

bool ReadVersion(int& nVersion)
Copy link
Member

@laanwj laanwj Mar 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

We probably want to keep this one around for symmetry.
Sometimes blindly removing unused code can cause an API to become lopsided, and cause methods to go missing which clearly will have a use at some point, and it just indicates more tests are needed. This seems applicable here.

@jonasschnelli
Copy link
Contributor

utACK 20da129626849511b41a870333dd0144eeaf0c6d, agree with @laanwj about ReadVersion().

maflcko
maflcko reviewed Mar 17, 2017
View reviewed changes
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK on removing the clearly outdated code (such as in test/) Not sure about the gains on removing some of the other methods.

src/chainparams.h
@@ -61,8 +61,6 @@ class CChainParams
int GetDefaultPort() const { return nDefaultPort; }

const CBlock& GenesisBlock() const { return genesis; }
/** Make miner wait to have peers to avoid wasting work */
bool MiningRequiresPeers() const { return fMiningRequiresPeers; }
Copy link
Member

Choose a reason for hiding this comment

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

Either keep everything or remove the member as well, no?

@practicalswift
Copy link
Contributor Author

practicalswift commented Mar 17, 2017 •
edited
Loading

@JeremyRubin @laanwj The changes to interpreter.cpp have now been reverted as suggested.

@laanwj @jonasschnelli ReadVersion(int& nVersion) is now kept around for symmetry.

@MarcoFalke Thanks for noticing! fMiningRequiresPeers has now been removed as well :-)

Thanks for reviewing! Please let me know if any further adjustments are needed.

dcousens
dcousens approved these changes Mar 17, 2017
View reviewed changes
Copy link
Contributor

@dcousens dcousens left a comment

Choose a reason for hiding this comment

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

re-ACK

@paveljanik
Copy link
Contributor

ACK 8dc957a

@laanwj laanwj merged commit 8dc957a into bitcoin:master Mar 18, 2017
laanwj added a commit that referenced this pull request Mar 18, 2017
@laanwj
Merge #9987: Remove unused code
aab1e55 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
@laanwj laanwj mentioned this pull request Mar 25, 2017
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 10, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
4f437bb 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 11, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
9727c3e 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 11, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
b3b69be 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 12, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
c6626ed 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 13, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
e36935e 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Mar 13, 2019
@laanwj @UdjinM6
Merge bitcoin#9987: Remove unused code
cb7a475 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 13, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
bce0a7a 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
1118527 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
32221d3 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 15, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
8ebb775 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 16, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
c3b425e 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 3, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
7849b42 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 3, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
e52bfb9 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 5, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
b90f0b1 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 6, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#9987: Remove unused code
eea8027 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
@laanwj
Merge bitcoin#9987: Remove unused code
dbdb201 
8dc957a Remove unused code (practicalswift)

Tree-SHA512: c7bb286e3b92e42fec8aa1ac2491fd38be36602efca16b4bdc4e9d5ada75c11d99e7713092ec13794abd69d5ef2c732b86209a6d01710e5ebf6fc51b8a65c92a

resolve usage of `MiningRequiresPeers()`

remove `fMiningRequiresPeers`
@practicalswift practicalswift deleted the remove-unused-code branch April 10, 2021 19:30
@str4d str4d mentioned this pull request Apr 12, 2021
Merged
@furszy furszy mentioned this pull request Sep 16, 2021
Merged
@barton2526 barton2526 mentioned this pull request Sep 22, 2021
Merged
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Sep 25, 2021
@furszy
Merge #2558: Cleaning unused code
1e2b533 
223bda1 key.cpp: Use PubKey constant size. (furszy)
0c6f95d remove unused code (furszy)

Pull request description:

  Straightforward unused code cleanup coming from bitcoin#9987 and bitcoin#10075.

ACKs for top commit:
  random-zebra:
    utACK 223bda1
  Fuzzbawls:
    ACK 223bda1

Tree-SHA512: 9937edbcf4f59ba7835f889ea4638d9bd32e28e3cd865ed043a1f0856ab8ec2b08a3cd4f851536db1703b7f21d4910f22fb905231f73dd0ae7adecd6f5a07055
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laanwj laanwj laanwj left review comments

+3 more reviewers

@maflcko maflcko maflcko left review comments

@paveljanik paveljanik paveljanik left review comments

@dcousens dcousens dcousens approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@practicalswift @NicolasDorier @JeremyRubin @laanwj @jonasschnelli @paveljanik @dcousens @maflcko @fanquake