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

Jump to bottom

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
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Milestone
Go1.8Early

Comments

@josharian
Copy link
Contributor

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

cc @bradfitz for opinions.
@bradfitz
Copy link
Contributor

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 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 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

I like it.

@robpike
Copy link
Contributor

robpike commented Mar 7, 2016

SGTM

@bradfitz bradfitz mentioned this issue Mar 7, 2016
Closed
@bradfitz bradfitz added the Suggested Issues that may be good for new contributors looking for work to do. label Apr 9, 2016
@bradfitz bradfitz added this to the Unplanned milestone Apr 9, 2016
@bradfitz bradfitz mentioned this issue May 10, 2016
Closed
@bradfitz bradfitz modified the milestones: Go1.8Early, Unplanned May 10, 2016
@bradfitz bradfitz self-assigned this May 10, 2016
@alexbrainman
Copy link
Member

SGTM

Alex

@bradfitz
Copy link
Contributor

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

@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.
Assignees
No one assigned
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Milestone
Go1.8Early
Development

No branches or pull requests
5 participants
@bradfitz @josharian @robpike @gopherbot @alexbrainman