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

Turn on warnings for stress client compilation. #96

Closed
srh opened this issue Nov 20, 2012 · 5 comments
Closed

Turn on warnings for stress client compilation. #96

srh opened this issue Nov 20, 2012 · 5 comments
Assignees
Milestone

Comments

@srh
Copy link
Contributor

srh commented Nov 20, 2012

While compiling the stress client on OS X, I got some warnings and realized that I'd be much more comfortable if -Werror was turned on, not to mention -Wall and -Wextra. This has a good chance of preventing wild goose chases when exhibiting bugs on platforms other than the commonly used ones.

@ghost ghost assigned srh Nov 20, 2012
@srh
Copy link
Contributor Author

srh commented Nov 21, 2012

Good news everybody. Lots of code in the stress client was just flat-out broken and has been caught thanks to turning on warnings.

@coffeemug
Copy link
Contributor

The stress client only works if you're very, very flexible about your
definition of the word "works". I wouldn't spend any time fixing it, it
basically needs to be rewritten in its entirety (or at the very least,
extremely heavily refactored).

On Tue, Nov 20, 2012 at 9:51 PM, srh notifications@github.com wrote:

Good news everybody. Lots of code in the stress client was just flat-out
broken and has been caught thanks to turning on warnings.


Reply to this email directly or view it on GitHubhttps://github.com//issues/96#issuecomment-10586400.

@srh
Copy link
Contributor Author

srh commented Nov 21, 2012

Too late. The changes in branch sam_96 are in review. It was not particularly difficult or time-consuming, and the benefit is that if we keep hacking at the existing stress client, we'll have warnings.

@coffeemug
Copy link
Contributor

@srh -- should this be closed?

@srh
Copy link
Contributor Author

srh commented Dec 3, 2012

Yes. Fixed as of 4081af8. Review 88.

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

No branches or pull requests

2 participants