Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Make helios-client guava 20 compatible. #1085

Merged
merged 1 commit into from
Feb 16, 2017

Conversation

daniel-kristjansson
Copy link

This changes some guava transform calls to transformAsync so we're compatible with guava 20.

We still pull in guava 19 for now.

This changes some guava transform calls to transformAsync so we're compatible with guava 20.
We still pull in guava 19 for now.
@davidxia davidxia closed this Feb 16, 2017
@davidxia davidxia reopened this Feb 16, 2017
@davidxia
Copy link
Contributor

why didn't tests run?

@davidxia
Copy link
Contributor

Tests don't seem to run for forked branches.

@mattnworb
Copy link
Member

@davidxia there is a setting in CircleCI for only building pull requests that come from the same repo, which makes sense security-wise in case your CircleCI build has any sensitive info in it like encrypted env vars, etc. I don't think we have any of those so the setting could probably be disabled.

@davidxia
Copy link
Contributor

@mattnworb Thanks. I've changed that.

@davidxia
Copy link
Contributor

@rohansingh Just curious. Any idea why we don't use AsyncFunction in HelisoClient in places like https://github.com/spotify/helios/blob/master/helios-client/src/main/java/com/spotify/helios/client/HeliosClient.java#L273?

Copy link
Member

@mattnworb mattnworb left a comment

Choose a reason for hiding this comment

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

@mattnworb
Copy link
Member

mattnworb commented Feb 16, 2017

@davidxia AsyncFunction is a function that returns a ListenableFuture<T>. If you don't have a future to return, then you just use Function. It is basically the same difference between thenApply and thenCompose with CompletableFutures.

@davidxia davidxia merged commit b7e791b into spotify:master Feb 16, 2017
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.

3 participants