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

Use LogPrintf instead of LogPrint #662

Closed
wants to merge 1 commit into from
Closed

Use LogPrintf instead of LogPrint #662

wants to merge 1 commit into from

Conversation

CryptoDJ
Copy link

No description provided.

@UdjinM6
Copy link

UdjinM6 commented Dec 21, 2015

Why?

PS. same question applies to #663

@CryptoDJ
Copy link
Author

For consistency since LogPrintf is used elsewhere in this method. When reading the code, I thought it was doing something different but when I looked at the definition, they are the same.

different issue: why is #define used instead of static const in masternode-budget.h and other places.
#define VOTE_ABSTAIN 0
should be changed to
static const uint32_t VOTE_ABSTAIN = 0;
Many recommend against using the preprocessor for constants (see Scott Meyers). If you agree they all should be changed to const, I will submit a pull.

@UdjinM6
Copy link

UdjinM6 commented Dec 21, 2015

Ah, I see. But no, they are different. Note that LogPrint here uses debug category mnbudget to suppress log output and prevent unwanted log spam unless you really want to see it (i.e. you are debugging budget messages with -debug=mnbudget cmd-line switch). LogPrintf however ignores categories so it will always write to debug.log which is no good in this particular case.

RE define->static: this part is off topic here but I agree we should name/use things in a more consistent way... blame @evan82 for this 😜 Not sure if it's critical though... I guess you can make a PR and we'll move this part of discussion there (let's see what others think about it).

@CryptoDJ
Copy link
Author

Okay, thank you and I will submit a separate pull request for the #define proposal. This masternode budget code is really impressive. @evan82, you and the rest of the Dash development team should get more recognition for this innovative technology.

@UdjinM6 UdjinM6 closed this Dec 21, 2015
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