Skip to content

Conversation

@jketema
Copy link
Contributor

@jketema jketema commented Dec 8, 2022

This brings the flow sources on par with isUserInput

I wasn't quite sure whether this should be a remote or local source. I picked remote, as getaddrinfo may involve a network lookup.

Disappointingly no new DCA results, contrary to the previous two PRs.

@github-actions github-actions bot added the C++ label Dec 8, 2022
@jketema jketema marked this pull request as ready for review December 8, 2022 22:36
@jketema jketema requested a review from a team as a code owner December 8, 2022 22:36
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

This LGTM!

I wasn't quite sure whether this should be a remote or local source. I picked remote, as getaddrinfo may involve a network lookup.

I think that's the right choice. We've had similar discussions with flow sources that are functions which take a file-descriptor argument (since that can be both a local file or a network socket). And for those, I seem to recall that we defaulted to making these remote flow sources.

@MathiasVP MathiasVP merged commit 7d1f10b into github:main Dec 9, 2022
@jketema jketema deleted the getaddrinfo branch December 9, 2022 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants