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

Do not prefix already prefixed format with 8 #48139

Merged
merged 5 commits into from
Oct 17, 2019

Conversation

pgomulka
Copy link
Contributor

It happens when a locale is specified on a mapping that a new
JavaDateFormatter is created with a new mapping and already prefixed
with 8 format is prefixed again. This should change avoids this
behavior.

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

It happens when a locale is specified on a mapping that a new
JavaDateFormatter is created with a new mapping and already prefixed
with 8 format is prefixed again. This should change avoids this
behavior.
@pgomulka pgomulka added :Core/Infra/Core Core issues without another label v6.8.5 labels Oct 16, 2019
@pgomulka pgomulka self-assigned this Oct 16, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

@pgomulka
Copy link
Contributor Author

discovered with yml test failure #47983

@pgomulka
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample-matrix

@pgomulka pgomulka requested a review from rjernst October 16, 2019 16:36
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Looks fine. Just to double check: we would have never serialized multiple 8's right?

@pgomulka
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@pgomulka
Copy link
Contributor Author

@rjernst good point, when serialising, it probably won't (I can't see how this could happen)
But when deserialising it would deserialise with double 8
added a testcase that fails without the change in JavaDateFormatter

@pgomulka pgomulka merged commit 1cc1885 into elastic:6.8 Oct 17, 2019
@jaymode jaymode added the >test Issues or PRs that are addressing/adding tests label Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v6.8.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants