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

Patch to histogram to allow zero fill to be turned off in the configuration #888

Closed
wants to merge 1 commit into from

Conversation

bwmeier
Copy link

@bwmeier bwmeier commented Jan 28, 2014

There's an existing patch to turn off zero-fill in the histogram inside the code, but it is not exposed in the configuration of the histogram panel. This patch adds a configuration option to the editor panel for the histogram to turn that off.

@rashidkpc
Copy link
Contributor

Looks good, have you completed step one of: https://github.com/elasticsearch/kibana/blob/master/CONTRIBUTING.md?

@bobrik
Copy link

bobrik commented Jan 29, 2014

Doesn't #612 do the same thing? Also, default isn't described, not sure if it's required.

@bwmeier
Copy link
Author

bwmeier commented Jan 29, 2014

@bobrik actually, these are different types of changes - the change in #612 appears to only affect the derivative views. My patch just exposes the zerofill option to the configuration UI. The default remains to zerofill the series, but now a user can control it. :-)

@bwmeier
Copy link
Author

bwmeier commented Jan 29, 2014

@rashidkpc I'm running the agreement by legal, I don't anticipate any issues.

@bobrik
Copy link

bobrik commented Jan 29, 2014

@bwmeier now I see your point, but #612 affects only non-derivative views just like your patch.

I just wonder what was wrong with #612, I'm not against your change at all.

@bwmeier
Copy link
Author

bwmeier commented Jan 29, 2014

@bobrik - Honestly, I didn't see #612 before I wrote this patch, which I implemented because I needed to show continuous trend graphs for metrics like CPU, memory and task duration. It looks like we just took different approaches: #612 is ignoring the zerofill option in the histogram panel and creating its own gapfill option, and I was just using the zerofill option as is and exposing it to the editor.

Essentially, both patches are expressing the need to be able to select between the four possible fill_style values: all, no, minimal, null (IMHO, these are not very descriptive of the actual style, but that's neither here nor there). I'll leave it up to @rashidkpc and team which way they want to implement that...

@bobrik
Copy link

bobrik commented Jan 29, 2014

@bwmeier there was no zerofill property at the time of #612 :)

@bwmeier
Copy link
Author

bwmeier commented Jan 29, 2014

@bobrik - makes sense. I'm not sure which is better overall, and honestly, I don't really care, as long as the functionality gets reflected one way or another - I just need smooth trendlines :-)

@bwmeier
Copy link
Author

bwmeier commented Feb 4, 2014

Legal is still reviewing the agreement, should be approved soon.

@gregberge
Copy link

+1 👍 for this issue !

@carguel
Copy link

carguel commented Mar 6, 2014

+1

1 similar comment
@Chadwiki
Copy link

+1

@rashidkpc
Copy link
Contributor

Since this appears to have stalled, I'm going to close it and see about merging #888

@rashidkpc rashidkpc closed this Mar 26, 2014
w33ble added a commit to w33ble/kibana that referenced this pull request Sep 13, 2018
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.

6 participants