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

missing date patch #4694

Merged
merged 4 commits into from Aug 19, 2015
Merged

missing date patch #4694

merged 4 commits into from Aug 19, 2015

Conversation

scampi
Copy link
Contributor

@scampi scampi commented Aug 18, 2015

closes #4693

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@rashidkpc rashidkpc added review bug Fixes for quality problems that affect the customer experience v4.2.0 and removed bug Fixes for quality problems that affect the customer experience labels Aug 18, 2015
@jbudz
Copy link
Member

jbudz commented Aug 18, 2015

Looks good @scampi! Two small concerns before merging:

  1. Can we make the output for null and undefined dates consistent - the empty string makes sense
  2. Add tests for converting null/undefined, https://github.com/elastic/kibana/blob/master/src/ui/public/stringify/__tests__/_string.js is a decent example to model off of

@scampi
Copy link
Contributor Author

scampi commented Aug 19, 2015

thanks for the review @jbudz. Both points should be ok now.

@jbudz
Copy link
Member

jbudz commented Aug 19, 2015

jenkins, test it

@jbudz
Copy link
Member

jbudz commented Aug 19, 2015

Thanks @scampi! One other issue, the test won't run unless it's loaded in https://github.com/elastic/kibana/blob/master/src/ui/public/stringify/__tests__/index.js

@scampi
Copy link
Contributor Author

scampi commented Aug 19, 2015

oops... added

@jbudz
Copy link
Member

jbudz commented Aug 19, 2015

jenkins, test it

@jbudz
Copy link
Member

jbudz commented Aug 19, 2015

LGTM

jbudz added a commit that referenced this pull request Aug 19, 2015
@jbudz jbudz merged commit c416a9f into elastic:master Aug 19, 2015
@scampi scampi deleted the undefined-date branch August 19, 2015 19:42
@scampi
Copy link
Contributor Author

scampi commented Aug 19, 2015

thanks @jbudz

jbudz added a commit that referenced this pull request Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

missing date field returns today's date
4 participants