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.

Sign up for GitHub

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

netx: refactor to reduce code duplication #310

Closed
bassosimone opened this issue Feb 1, 2020 · 0 comments · Fixed by #382
Closed

netx: refactor to reduce code duplication #310

bassosimone opened this issue Feb 1, 2020 · 0 comments · Fixed by #382
Assignees
@bassosimone
Labels
bug Something isn't working enhancement New feature or request ooni/netx Issues related to github.com/ooni/netx priority/medium Medium priority

Comments

@bassosimone
Copy link
Contributor

There is duplication in netx. The internal/internal.go code is the real implementation. netx.go and httpx/httpx.go duplicate code and tests that are inside internal. This is caused by the fact that I chose to split netx and httpx in two different packages. However, this split in two packages is just an annoyance, precisely because of duplicated tests, etc., and we should just remove that.

This insight occurred to me while reasoning about how to approach #229.
@bassosimone bassosimone added bug Something isn't working enhancement New feature or request priority/medium Medium priority effort/S Small effort ooni/netx Issues related to github.com/ooni/netx labels Feb 1, 2020
@bassosimone bassosimone added this to the Sprint 7 - Crown jellyfish milestone Feb 1, 2020
@bassosimone bassosimone self-assigned this Feb 1, 2020
bassosimone added a commit that referenced this issue Mar 4, 2020
@bassosimone
netx: merge public API with internal
f7b08ca 
Closes #338, because I have
added code and unit test to check for such case.

Closes #310.

Part of #302.
@bassosimone bassosimone mentioned this issue Mar 4, 2020
Merged
bassosimone added a commit that referenced this issue Mar 4, 2020
@bassosimone
netx: merge public API with internal (#382)
658a781 
Closes #338, because I have
added code and unit test to check for such case.

Closes #310.

Part of #302.
@bassosimone bassosimone removed the effort/S Small effort label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
bug Something isn't working enhancement New feature or request ooni/netx Issues related to github.com/ooni/netx priority/medium Medium priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

netx: merge public API with internal ooni/probe-engine
1 participant
@bassosimone