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 commented out code #3117

Merged
merged 3 commits into from
Sep 30, 2019

Conversation

PastaPastaPasta
Copy link
Member

We have version control systems for a reason, if we want code to not run it should be removed. I personally see no value in keeping these around. I presume at one point they were spamming debug.log so we commented them out, but we really should have just removed them.

I believe all of this is dash specific code but any conflicts this does create are so minor they are not of concern imo.

Signed-off-by: Pasta [email protected]

We have version control systems for a reason, if we want code to not run it should be removed. I personally see no value in keeping these around. I presume at one point they were spamming debug.log so we commented them out, but we really should have just removed them.

I believe all of this is dash specific code but any conflicts this does create are so minor they are not of concern imo.

Signed-off-by: Pasta <[email protected]>
UdjinM6
UdjinM6 previously approved these changes Sep 27, 2019
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 added this to the 14.1 milestone Sep 27, 2019
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

See inline comments

src/keepass.cpp Outdated
@@ -341,7 +341,6 @@ void CKeePassIntegrator::doHTTPPost(const std::string& sRequest, int& nStatusRet
evhttp_add_header(output_headers, "Connection", "close");

// Logging of actual post data disabled as to not write passphrase in debug.log. Only enable temporarily when needed

Choose a reason for hiding this comment

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

Maybe also remove this comment as it was referring to the commented out log print

src/keepass.cpp Outdated
@@ -452,7 +451,6 @@ std::vector<CKeePassIntegrator::CKeePassEntry> CKeePassIntegrator::rpcGetLogins(
doHTTPPost(request.getJson(), nStatus, strResponse);

// Logging of actual response data disabled as to not write passphrase in debug.log. Only enable temporarily when needed

Choose a reason for hiding this comment

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

same

@@ -224,8 +224,6 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
// Update coinbase transaction with additional info about masternode and governance payments,
// get some info back to pass to getblocktemplate
FillBlockPayments(coinbaseTx, nHeight, blockReward, pblocktemplate->voutMasternodePayments, pblocktemplate->voutSuperblockPayments);
// LogPrintf("CreateNewBlock -- nBlockHeight %d blockReward %lld txoutMasternode %s coinbaseTx %s",
// nHeight, blockReward, pblocktemplate->txoutsMasternode.ToString(), coinbaseTx.ToString());

Choose a reason for hiding this comment

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

This probably wouldn't even compile anymore 🙈

nmarley
nmarley previously approved these changes Sep 27, 2019
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

Not sure it hurts much, but do agree w/removing unused code.

utACK

@PastaPastaPasta PastaPastaPasta dismissed stale reviews from nmarley and UdjinM6 via 93b20c3 September 28, 2019 17:17
@PastaPastaPasta
Copy link
Member Author

resolved @codablock's suggestions, also removed some extra code in keepass.cpp which was commented out

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

re-utACK

nit: PR title needs update now :)

@PastaPastaPasta PastaPastaPasta changed the title Remove LogPrints which have been commented out. Remove commented out code. Sep 28, 2019
@nmarley nmarley changed the title Remove commented out code. Remove commented out code Sep 28, 2019
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

utACK

@codablock codablock merged commit 746b5f8 into dashpay:develop Sep 30, 2019
@PastaPastaPasta PastaPastaPasta deleted the remove-commented-out-logs branch September 30, 2019 14:14
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
* Remove LogPrints which have been commented out.

We have version control systems for a reason, if we want code to not run it should be removed. I personally see no value in keeping these around. I presume at one point they were spamming debug.log so we commented them out, but we really should have just removed them.

I believe all of this is dash specific code but any conflicts this does create are so minor they are not of concern imo.

Signed-off-by: Pasta <[email protected]>

* remove a couple of extra comments

Signed-off-by: Pasta <[email protected]>

* remove commented out code

Signed-off-by: Pasta <[email protected]>
panleone pushed a commit to panleone/PIVX that referenced this pull request Nov 15, 2024
panleone pushed a commit to panleone/PIVX that referenced this pull request Nov 15, 2024
Fuzzbawls added a commit to PIVX-Project/PIVX that referenced this pull request Jan 11, 2025
a85b450 Merge pull request dashpay#3399 from codablock/pr_speedups2 (Alexander Block)
136f900 cherry-pick dashpay#2833 (Alexander Block)
d942439 Merge pull request dashpay#3389 from codablock/pr_concentrated_recovery (Alexander Block)
36790d2 cherry pick dashpay#3368 (Author Alexander Block)
dac01a9 cherry-pick dashpay#2780 (Alexander Block)
39d0ed9 cherry-pick dashpay#3367 (Alexander Block)
5084bbf Allow re-signing of IS locks when performing retroactive signing (dashpay#3219) (Alexander Block)
802c006 Only track last seen time instead of first and last seen time (dashpay#3165) (Alexander Block)
479b64b Avoid propagating InstantSend related old recovered sigs (dashpay#3145) (Alexander Block)
27fa2af cherry-pick dashpay#3117 (Pasta)
cf138e0 cherry-pick dashpay#3097 (Pasta)
23b140e Introduce getbestchainlock rpc and fix llmq-is-cl-conflicts.py (dashpay#3094) (UdjinM6)

Pull request description:

  as usual each commit backports a different PR

ACKs for top commit: a85b450
  Duddino:
    utACK a85b450
  Liquid369:
    uTACK a85b450
  Fuzzbawls:
    ACK a85b450

Tree-SHA512: e9024d180888d8a6cc300ba9df74fc15929e3ade1773e5d312bd8cc93f6c9fd3898c5bf2d14672abf4faba576575c33936708e6e1dfd01a393479d264d3f2c57
Liquid369 pushed a commit to Liquid369/PIVX that referenced this pull request Jan 14, 2025
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