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

[DOCS] Fix heading format errors #31483

Merged
merged 3 commits into from
Jun 26, 2018
Merged

[DOCS] Fix heading format errors #31483

merged 3 commits into from
Jun 26, 2018

Conversation

Sue-Gallagher
Copy link
Contributor

@Sue-Gallagher Sue-Gallagher commented Jun 20, 2018

Affects composite-aggregation.asciidoc - Closes #31327

@Sue-Gallagher Sue-Gallagher requested a review from lcawl June 20, 2018 22:41
@colings86 colings86 added >docs General docs changes :Analytics/Aggregations Aggregations labels Jun 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@nik9000 nik9000 changed the title [DOCS] Fix heading format errors. Closes #31327 [DOCS] Fix heading format errors Jun 21, 2018
@@ -224,7 +224,7 @@ Time values can also be specified via abbreviations supported by <<time-units,ti
Note that fractional time values are not supported, but you can address this by shifting to another
time unit (e.g., `1.5h` could instead be specified as `90m`).

====== Format
===== Format
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this section is supposed to be a subsection of the Date Histogram section though. Same with the one below it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right; it should be. But the software is set up to only allow so many heading levels, and there's nothing I can do to change that. As it is, it's nested too far down to appear in the TOC, so the exact level doesn't much matter. If you prefer, I can make it a normal paragraph with bold text so it will look more like the heading level it should be.

Copy link
Contributor

@lcawl lcawl Jun 21, 2018

Choose a reason for hiding this comment

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

Maybe a definition list would work too? I just ran a quick test and it works, though the content is indented, so that might not be what you want.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure how to format it. I'd try just removing the heading all together and see how it reads. Bold text'd be fine with me too. Definition list feels a little lick a hack but, sometimes a hack is the right way to go. I'm not sure!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the input. Going with the simple solution - bold text (composite-aggregation.asciidoc)

@Sue-Gallagher Sue-Gallagher merged commit 357a07e into master Jun 26, 2018
@Sue-Gallagher Sue-Gallagher deleted the formatfix branch June 26, 2018 00:25
dnhatn added a commit that referenced this pull request Jun 26, 2018
* master:
  ingest: Add ignore_missing property to foreach filter (#22147) (#31578)
  Fix a formatting issue in the docvalue_fields documentation. (#31563)
  reduce log level at gradle configuration time
  [TEST] Close additional clients created while running yaml tests (#31575)
  Docs: Clarify sensitive fields watcher encryption (#31551)
  Watcher: Remove never executed code (#31135)
  Add support for switching distribution for all integration tests (#30874)
  Improve robustness of geo shape parser for malformed shapes (#31449)
  QA: Create xpack yaml features (#31403)
  Improve test times for tests using `RandomObjects::addFields` (#31556)
  [Test] Add full cluster restart test for Rollup (#31533)
  Enhance thread context uniqueness assertion
  [DOCS] Fix heading format errors (#31483)
  fix writeIndex evaluation for aliases (#31562)
  Add x-opaque-id to search slow logs (#31539)
  Watcher: Fix put watch action (#31524)
  Add package pre-install check for java binary (#31343)
  Reduce number of raw types warnings (#31523)
  Migrate scripted metric aggregation scripts to ScriptContext design (#30111)
  turn GetFieldMappingsResponse to ToXContentObject (#31544)
  Close xcontent parsers (partial) (#31513)
  Ingest Attachment: Upgrade Tika to 1.18 (#31252)
  TEST: Correct the assertion arguments order (#31540)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >docs General docs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants