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

Minor - Wrong Code in Cookbook documentation #246

Closed
madhava-sridhar opened this issue Apr 11, 2019 · 3 comments
Closed

Minor - Wrong Code in Cookbook documentation #246

madhava-sridhar opened this issue Apr 11, 2019 · 3 comments
Labels
good first issue

Comments

@madhava-sridhar
Copy link

madhava-sridhar commented Apr 11, 2019 •
edited
Loading

Wrong Code in Cookbook documentation

Expected behavior

In page https://twitter.github.io/util/guide/util-cookbook/futures.html , in 1st Code example method "asScala" has param "implicit e: ExecutionContext " which is not required by code contained in the method "asScala" (Execution Context is not used by contained code as well)

Actual behavior

method "asScala" doesn't have param "implicit e: ExecutionContext "

Steps to reproduce the behavior

NA
@kevinoliver
Copy link
Contributor

yep, good eyes.

any interest in putting together the small PR to fix?

@soojee
Copy link
Contributor

soojee commented Apr 14, 2019

Hi, I'd like to take a go at this!

@soojee soojee mentioned this issue Apr 15, 2019
Closed
soojee added a commit to soojee/util that referenced this issue Apr 16, 2019
@soojee
doc: Remove unused parameter in Future conversion example
a8f2776 
Problem

The asScala method in the first example of the Futures documentation
passes in an unused parameter, implicit e: ExecutionContext.

Solution

Removed unused parameter.

Github issue twitter#246
finaglehelper pushed a commit that referenced this issue Apr 17, 2019
@soojee
doc: Remove unused parameter in Future conversion example
78a0310 
Problem

The asScala method in the first example of the Futures documentation
passes in an unused parameter, implicit e: ExecutionContext.

Solution

Removed unused parameter.

Github issue #246

Signed-off-by: Yufan Gong <yufang@twitter.com>

Differential Revision: https://phabricator.twitter.biz/D301848
@soojee
Copy link
Contributor

soojee commented Apr 17, 2019

Fixed by #247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: Remove unused parameter in Future conversion example soojee/util
4 participants
@kevinoliver @madhava-sridhar @yufangong @soojee