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

FunctionScore: RandomScoreFunction now accepts long, as well a strings. #8311

Closed
wants to merge 3 commits into from

Conversation

@rjernst
Copy link
Member

commented Oct 31, 2014

closes #8267

}

/**
* seed variant taking a long value.

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 31, 2014

Contributor

this javadoc is wrong

This comment has been minimized.

Copy link
@rjernst

rjernst Oct 31, 2014

Author Member

Fixed.

@@ -28,7 +28,7 @@
*/
public class RandomScoreFunctionBuilder extends ScoreFunctionBuilder {

private Integer seed = null;
private Object seed = null;

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 31, 2014

Contributor

maybe we can just make this Number and we try to parse the string?

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 31, 2014

Contributor

nevermind I saw the usage

} else if (seed instanceof Long) {
builder.field("seed", ((Long)seed).longValue());
} else if (seed instanceof String) {
builder.field("seed", (String)seed);

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 31, 2014

Contributor

I think you should just do instanceof Number and else call .toString()

This comment has been minimized.

Copy link
@rjernst

rjernst Oct 31, 2014

Author Member

I was trying to satisfy the possibility for a session id (say alphanum + dashes).

This comment has been minimized.

Copy link
@s1monw

s1monw Oct 31, 2014

Contributor

I modified the comment after you replied ^^

This comment has been minimized.

Copy link
@s1monw

s1monw Nov 3, 2014

Contributor

@rjernst can you fix this ^^

This comment has been minimized.

Copy link
@rjernst

rjernst Nov 3, 2014

Author Member

Oops, sorry, I misunderstood. Fixed.

@rjernst

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2014

@s1monw anything more here?

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2014

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.