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

Whitelist expansion #18372

Merged
merged 1 commit into from May 16, 2016

Conversation

Projects
None yet
4 participants
@rmuir
Copy link
Contributor

commented May 16, 2016

Most of these improvements focus on the basic types:

  • add compare/compareTo/toHexString/parseX for basic types
  • add Integer/Long/Float/Double min/max and remove the imax/lmax stuff
  • add Character methods like isWhiteSpace and similar
  • add Math E and PI constants.

We need to followup with docs improvements (its already out of date anyway). In general I think the way we present that needs to be re-organized completely. There is a lot more to do here, but these changes are all simple and easy.

@jdconrad

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

LGTM. Thanks for adding more!

@uschindler

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

Do we also have some real read/write on fields, so we can check painless DefField class? Static fields don't help... Currently there is no way to test the code!

@rmuir

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2016

This PR is really just to capture "easy wins". I know the problem there, but we should solve it differently IMO. I also omitted any date apis or similar here for the same reason. They are more complex.

@jdconrad

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

@uschindler So there were actually tests for this before, but they got removed when we decided to make Definition a singleton. It should be possible to have a dummy dictionary again for field tests, but this should probably be a separate issue.

@rmuir rmuir merged commit e69305a into elastic:master May 16, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details
@uschindler

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

@jdconrad thanks for this. I think this is ok for now. I'd suggest to add some "undocumented" fake class for testing that is added to the Definition.

@clintongormley clintongormley changed the title painless whitelist expansion Whitelist expansion May 17, 2016

@jdconrad jdconrad referenced this pull request May 17, 2016

Closed

Ongoing Painless Improvements #17992

15 of 18 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.