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

Fix -Wstring-plus-int warning on clang #3227

Merged
merged 1 commit into from May 3, 2018

Conversation

5 participants
@str4d
Contributor

str4d commented May 1, 2018

No description provided.

@str4d str4d added this to the v1.1.1 milestone May 1, 2018

@str4d str4d requested review from gtank and Eirik0 May 1, 2018

@Eirik0

Eirik0 approved these changes May 1, 2018

ACK

@str4d str4d added this to In Review in Consensus Protocol Team May 1, 2018

@Empact

This comment has been minimized.

Empact commented May 2, 2018

The error is more informative with the type code. How about including it via strprintf or the like?

@Eirik0

This comment has been minimized.

Contributor

Eirik0 commented May 3, 2018

@Empact I thought about this too, but I am not sure that that would be a more useful to the end user or the developer who was fixing whatever bug was introduced that caused this to happen. Also this code should never actually be executed, and I think that this would be an unlikely bug to be introduced.

@gtank

gtank approved these changes May 3, 2018

With the caveat that I don't understand what would even cause this error to fire, LGTM.

@str4d

This comment has been minimized.

Contributor

str4d commented May 3, 2018

The only time anyone will hit the default case is if they are implementing a new type, at which point the cause will be obvious.

@zkbot r+

@zkbot

This comment has been minimized.

Contributor

zkbot commented May 3, 2018

📌 Commit 1f9dfbb has been approved by str4d

@zkbot

This comment has been minimized.

Contributor

zkbot commented May 3, 2018

⌛️ Testing commit 1f9dfbb with merge c7f5d5c...

zkbot added a commit that referenced this pull request May 3, 2018

Auto merge of #3227 - str4d:3191-nullifier-macos-fix, r=str4d
Fix -Wstring-plus-int warning on clang
@zkbot

This comment has been minimized.

Contributor

zkbot commented May 3, 2018

☀️ Test successful - pr-merge
Approved by: str4d
Pushing c7f5d5c to master...

@zkbot zkbot merged commit 1f9dfbb into zcash:master May 3, 2018

1 check passed

homu Test successful
Details

Consensus Protocol Team automation moved this from In Review to Complete PRs (Ignore) May 3, 2018

@str4d str4d deleted the str4d:3191-nullifier-macos-fix branch May 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment