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

Apptesting: Change Interface to Struct #1046

Merged
merged 3 commits into from
Mar 7, 2022
Merged

Conversation

mattverse
Copy link
Member

Description

Extension for pull #1008.
We should compare this with the interface approach and compare on usability


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@mattverse mattverse requested a review from ValarDragon March 5, 2022 12:47
@mattverse mattverse marked this pull request as draft March 5, 2022 12:47
@mattverse
Copy link
Member Author

@ValarDragon this is just a draft as of now, can you review and see if this was similar to the approach you were thinking of?

@mattverse mattverse requested a review from ValarDragon March 7, 2022 03:54
@mattverse mattverse marked this pull request as ready for review March 7, 2022 03:56
@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2022

Codecov Report

Merging #1046 (9a74aa5) into main (41a0a0b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1046   +/-   ##
=======================================
  Coverage   20.60%   20.60%           
=======================================
  Files         193      193           
  Lines       25237    25237           
=======================================
  Hits         5199     5199           
  Misses      19084    19084           
  Partials      954      954           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41a0a0b...9a74aa5. Read the comment docs.

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM! I like the struct approach much more than the initial interface idea

@ValarDragon ValarDragon merged commit b27b5b2 into main Mar 7, 2022
@ValarDragon ValarDragon deleted the mattverse/app-testing branch March 7, 2022 16:50
@ValarDragon ValarDragon added the A:backport/v7.x Do not use. backport patches to v7.x branch label Mar 7, 2022
mergify bot pushed a commit that referenced this pull request Mar 7, 2022
* Initial Commit

* Delete unnecessary lines

* Embed suite and keeper helper

(cherry picked from commit b27b5b2)
ValarDragon pushed a commit that referenced this pull request Mar 7, 2022
* Initial Commit

* Delete unnecessary lines

* Embed suite and keeper helper

(cherry picked from commit b27b5b2)

Co-authored-by: Matt, Park <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v7.x Do not use. backport patches to v7.x branch
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants