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

[API CHANGE] Refactor code for fetching test lists #156

Merged
merged 5 commits into from
Dec 8, 2019

Conversation

bassosimone
Copy link
Contributor

  1. using a "client" for a test lists query is overkill, instead just
    have a single function with config and possibly result (the same
    concept can be applied to other places in the codebase, in which we
    create "client" instances we don't actually need)

  2. make sure the toplevel API can be used from mobile code, i.e., avoid
    using a vector that gomobile cannot bind

  3. verify, by looking at changed tests and miniooni, that the result
    API is still reasonably non-painful to use

  4. make sure that the internal implementation details are part of the
    internal package: we're currently exposing too much internals

These changes are yak shaving I implemented while starting the work
to implement authenticated request for orchestra (i.e. #148)

1. using a "client" for a test lists query is overkill, instead just
have a single function with config and possibly result (the same
concept can be applied to other places in the codebase, in which we
create "client" instances we don't actually need)

2. make sure the toplevel API can be used from mobile code, i.e., avoid
using a vector that gomobile cannot bind

3. verify, by looking at changed tests and miniooni, that the result
API is still reasonably non-painful to use

4. make sure that the internal implementation details are part of the
internal package: we're currently exposing too much internals

These changes are yak shaving I implemented while starting the work
to implement authenticated request for orchestra (i.e. #148)
@bassosimone bassosimone marked this pull request as ready for review December 7, 2019 16:17
@bassosimone bassosimone self-assigned this Dec 7, 2019
@bassosimone
Copy link
Contributor Author

I am going to merge with failing Travis because the failed build is useful to investigate #142.

@bassosimone bassosimone merged commit f13e4e1 into master Dec 8, 2019
@bassosimone bassosimone deleted the refactor/testlists branch December 8, 2019 14:15
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