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

Suppress C++ compiler warnings #754

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Suppress C++ compiler warnings #754

wants to merge 2 commits into from

Conversation

lgeiger
Copy link
Member

@lgeiger lgeiger commented Sep 12, 2022

What do these changes do?

This PR suppresses C++ warnings since our build since the CI build logs are getting really long. This is copied directly from the TensorFlow bazel config. Not sure whether this ignores any important warnings, but to be honest the logs are so messy that it is almost impossible to see something in there anyway.

How Has This Been Tested?

I tested this with a full converter build and it reduces the logged lines in GitHub actions from 19373 to 339.

@lgeiger lgeiger added the internal-improvement Internal Improvements and Maintenance label Sep 12, 2022
@lgeiger lgeiger requested a review from a team September 12, 2022 09:58
@lgeiger
Copy link
Member Author

lgeiger commented Sep 12, 2022

Looking at TensorFlow's bazel config again, it seems like they specifically enable some warnings on Linux. Should we do the same?

@Tombana
Copy link
Collaborator

Tombana commented Sep 12, 2022

Looking at TensorFlow's bazel config again, it seems like they specifically enable some warnings on Linux. Should we do the same?

I think that on CI it is indeed a bit useless to have any warnings printed, since there are so many and most of them are not from our code. If it would be possible to only get warnings for everyhing in the larq_compute_engine dir, that might be a nice improvement. It might also be nice to make this "no warnings" option something for CI only, so that local runs still have all/most warnings. To achieve that maybe we have to add these no-warning flags in the github actions bazel setup script, i.e. echo "build:linux -no_warnings" >> .bazelrc.user or something. Or maybe make a build:ci_no_warnings config where we disable the warnings, and then in the github actions file add --config=ci_no_warnings. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-improvement Internal Improvements and Maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants