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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix 2612 #2613

Merged
merged 1 commit into from
Jan 7, 2019
Merged

Fix 2612 #2613

merged 1 commit into from
Jan 7, 2019

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jan 7, 2019

No idea how I missed this 馃檲

@UdjinM6 UdjinM6 added this to the 14.0 milestone Jan 7, 2019
PastaPastaPasta
PastaPastaPasta reviewed Jan 7, 2019
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

Doesn't technically matter as NULL == 0

@nmarley
Copy link

nmarley commented Jan 7, 2019

Doesn't technically matter as NULL == 0

This is not a good pattern to follow, for reasons listed in the book we've discussed 1:1. Technically that's an implementation detail and should not be used just because "it works".

The macro NULL is an implementation-defined C++ null pointer constant in this International Standard (4.10).180)

nmarley
nmarley approved these changes Jan 7, 2019
View reviewed changes
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.

utACK

@UdjinM6
Copy link
Author

UdjinM6 commented Jan 7, 2019

Agree with @nmarley - no need for inconsistent and confusing stuff (and compiler warnings ;) ).

codablock
codablock approved these changes Jan 7, 2019
View reviewed changes
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

@UdjinM6 UdjinM6 merged commit bade332 into dashpay:develop Jan 7, 2019
@PastaPastaPasta
Copy link
Member

Got it 馃憤

@PastaPastaPasta PastaPastaPasta mentioned this pull request Jan 31, 2019
Merged
@UdjinM6 UdjinM6 deleted the fix2612 branch November 26, 2020 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PastaPastaPasta PastaPastaPasta PastaPastaPasta left review comments

@nmarley nmarley nmarley approved these changes

@codablock codablock codablock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@UdjinM6 @nmarley @PastaPastaPasta @codablock