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

Remove unused locks and consolidate test helpers. #6928

Merged
merged 1 commit into from Sep 4, 2019

Conversation

@raskchanky
Copy link
Member

commented Sep 3, 2019

The OutputBuffer construct that was used to test certain functions
involving the UI struct was entirely unnecessary, given UI::with_sinks.

This is in service of completing #6435

Signed-off-by: Josh Black raskchanky@gmail.com

@chef-expeditor

This comment has been minimized.

Copy link

commented Sep 3, 2019

Hello raskchanky! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

Copy link
Contributor

left a comment

This is awesome!

I think we can go ever a bit further and eliminate OutputBuffer (and the test_helpers module). All three instances use OutputBuffer to define the same ui() -> (UI, OutputBuffer, OutputBuffer) function. But it's just for the sake of fulfilling the UI contract; the OutputBuffers are never read from, so I think we can eliminate these ui functions entirely and call the already existing UI::with_sinks for the tests.

@raskchanky

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@baumanj Good point - I didn't even know about with_sinks. I'll do that.

@raskchanky raskchanky force-pushed the jb/output-buffer-locks branch from 28d54bf to 03ff50c Sep 4, 2019
The OutputBuffer construct was entirely unnecessary, given the presence
of UI::with_sinks.

Signed-off-by: Josh Black <raskchanky@gmail.com>
@raskchanky raskchanky force-pushed the jb/output-buffer-locks branch from 03ff50c to 2ee3815 Sep 4, 2019
@baumanj
baumanj approved these changes Sep 4, 2019
@raskchanky raskchanky merged commit 8dbc263 into master Sep 4, 2019
5 checks passed
5 checks passed
DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #3312 passed (21 minutes, 11 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #431 passed (48 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details
@raskchanky raskchanky deleted the jb/output-buffer-locks branch Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.