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

Remove unnecessary condition in _safe_flags function #132

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

reimerix
Copy link

@reimerix reimerix commented Apr 4, 2024

Description

Cover the following topics in the description:

  • Rational
  • Context
  • Design notes

Jira Reference

Link to a Jira ticket:

@reimerix reimerix requested a review from a team as a code owner April 4, 2024 22:22
Copy link
Contributor

@jungleraptor jungleraptor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably added this to work around the fact that bazel doesn't distinguish between options for c++ or c files, so we were running into issues where it would pass -std=c++14 (or similiar), when compiling a c file, and that would error because we're building with -Werror (warning that the option is not recognized for c files).

But we figured out how to work around that quite a while ago so we probably just forgot to remove this hack.

Assuming this doesn't break anything I think we should definitely take this fix.

Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isaactorz @swift-nav/internal-ops

-- I think this is another place we should consider removing the devinfra team from CodeOwners

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

Successfully merging this pull request may close these issues.

None yet

3 participants