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

cleanup(netxlite): remove first-order implicit-Netx wrappers #1465

Merged
merged 13 commits into from
Jan 24, 2024

Conversation

bassosimone
Copy link
Contributor

This diff removes the first-order implicit-Netx wrappers. We define first-order implicit-Netx wrappers the top-level functions that have the same name of netxlite.Netx methods, allocate an empty Netx, and call the corresponding method.

The reason why we're doing this now is that it has been relatively hard to implement #1464 because of the ambiguity between those first-order wrappers and the methods. Getting this wrong means that QA tests would behave in a funny way.

The overall goal here is to refactor oohelperd to only depend on netxlite.Netx such that we can remove the code duplication between how we instantiate oohelperd.Handler in oohelperd and how we instantiate it inside netemx.

In turn, by doing this, we would ensure we have the same oohelperd behavior for QA and production.

In turn, with this guarantee, we can write QA tests that ensure we're correctly dealing with 127.0.0.1.

The reference issue is ooni/probe#1517.

@bassosimone bassosimone requested a review from hellais as a code owner January 24, 2024 09:46
@bassosimone bassosimone merged commit 70861a6 into master Jan 24, 2024
9 of 11 checks passed
@bassosimone bassosimone deleted the issue/1517c branch January 24, 2024 09:55
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this pull request Feb 13, 2024
This diff removes the first-order implicit-Netx wrappers. We define
first-order implicit-Netx wrappers the top-level functions that have the
same name of netxlite.Netx methods, allocate an empty Netx, and call the
corresponding method.

The reason why we're doing this now is that it has been relatively hard
to implement ooni#1464 because of the
ambiguity between those first-order wrappers and the methods. Getting
this wrong means that QA tests would behave in a funny way.

The overall goal here is to refactor `oohelperd` to only depend on
`netxlite.Netx` such that we can remove the code duplication between how
we instantiate `oohelperd.Handler` in `oohelperd` and how we instantiate
it inside `netemx`.

In turn, by doing this, we would ensure we have the same `oohelperd`
behavior for QA and production.

In turn, with this guarantee, we can write QA tests that ensure we're
correctly dealing with 127.0.0.1.

The reference issue is ooni/probe#1517.
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.

1 participant