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

Optimist tests cleanup #2 #3834

Merged
merged 10 commits into from
Oct 27, 2018

Conversation

joshribakoff
Copy link
Contributor

As discussed in #3712 this is a continuation of PR #3713 further cleaning up the tests

@joshribakoff joshribakoff changed the title WIP: Optimist tests cleanup #2 Optimist tests cleanup #2 Aug 18, 2018
@joshribakoff
Copy link
Contributor Author

This PR is complete.

In my next PR, I will most likely extract test helpers http://xunitpatterns.com/Test%20Helper.html, several rounds of flattening after that would most likely be called for, before I attempt to write failing tests for & fix the underlying bug.

My goal is 4-7 LOC for each test in this file.

@joshribakoff
Copy link
Contributor Author

This article might help filter the white space changes, only a few lines actually changed. https://michaelheap.com/a-better-git-diff/

@hwillson hwillson self-assigned this Oct 3, 2018
@joshribakoff
Copy link
Contributor Author

Is there anything I can do to expedite this? Can I work on any bug fixes or code reviews that would free up more of your time to review this? Would I be able to send bigger PRs, or a larger change set split up into multiple PRs but all at once? I feel like I could greatly simplify these tests, and I could move much more quickly & the code sitting in PR review is the only bottleneck here from my perspective.

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Thanks very much @joshribakoff!

@hwillson hwillson merged commit e19908f into apollographql:master Oct 27, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants