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

cmd/dist: should C warnings cause make.bash to fail? #14698

Closed
josharian opened this issue Mar 7, 2016 · 7 comments
Closed

cmd/dist: should C warnings cause make.bash to fail? #14698

josharian opened this issue Mar 7, 2016 · 7 comments
Assignees
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Mar 7, 2016

The warning described in #14696 did not cause the build to fail. Should it have?

cc @bradfitz for opinions.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 7, 2016

I'd be in favor of making C warnings fatal during make.bash.

@robpike, @rsc, @ianlancetaylor ... ?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 7, 2016

Then again, C compilers add warnings all the time, and we were already bitten by that recently with a527bdb for #12345

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Mar 7, 2016

Talking to myself, maybe we only make them fatal for dev builds, or only when run under the builders, so end users with release builds are never affected by the latest and greatest C warnings added in C compilers newer than what we used during development.

@josharian
Copy link
Contributor Author

@josharian josharian commented Mar 7, 2016

I like it.

@robpike
Copy link
Contributor

@robpike robpike commented Mar 7, 2016

SGTM

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented May 10, 2016

SGTM

Alex

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 29, 2016

Gopherbot never mentioned this, probably because it says Do Not Review, but: https://go-review.googlesource.com/#/c/23005/

@gopherbot gopherbot closed this in b6e3a98 Aug 30, 2016
@golang golang locked and limited conversation to collaborators Aug 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.