Skip to content

Fixing URL to Grok patterns#29088

Merged
cbuescher merged 1 commit intoelastic:masterfrom
jtyr:jtyr-doc_grok_patterns
Mar 16, 2018
Merged

Fixing URL to Grok patterns#29088
cbuescher merged 1 commit intoelastic:masterfrom
jtyr:jtyr-doc_grok_patterns

Conversation

@jtyr
Copy link
Copy Markdown
Contributor

@jtyr jtyr commented Mar 15, 2018

This PR is just fixing the link to the Grok reusable patterns.

@elasticmachine
Copy link
Copy Markdown
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Copy Markdown
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cbuescher cbuescher added >docs General docs changes review :Distributed/Ingest Node Execution or management of Ingest Pipelines v7.0.0 labels Mar 15, 2018
Copy link
Copy Markdown
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@jtyr thanks a lot for fixing the broken link, I left a small suggestion for improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: a slight improvement would be to link to the grok-patterns file on the current branch (either master or one of the version branches, e.g. 6.x, 6.2). You can use the {branch} placeholder for this inside the link as I just learned recently

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion. It's fixed now.

@jtyr jtyr force-pushed the jtyr-doc_grok_patterns branch from 54c90ce to 5bce41a Compare March 15, 2018 16:42
@cbuescher cbuescher self-assigned this Mar 15, 2018
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry about nitpicking, but when I re-checked the target directory for the link, I saw that maybe the parent directory (https://github.com/elastic/elasticsearch/tree/master/libs/grok/src/main/resources/patterns) would be a better choice? I think the link originally pointed there (e.g. on 5.6 branch: https://github.com/elastic/elasticsearch/tree/5.6/modules/ingest-common/src/main/resources/patterns)
Maybe @mvg would also know if the number of patterns should be adapted while we are at it. Looks like way more thank 120 to me, are they all packaged with the grok processor or only a subset of them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right. The link should point into the directory and not to the file. I have fixed that and I have also changed the wording a little bit to be more future-proof.

@cbuescher
Copy link
Copy Markdown
Member

@jtyr thanks for the update, I was about to merge but found another tiny thing to at least check while changing this, left a question for @mvg for clarification

@jtyr jtyr force-pushed the jtyr-doc_grok_patterns branch from 5bce41a to a04906f Compare March 16, 2018 09:58
Copy link
Copy Markdown
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM
Will run final tests before merging.

@cbuescher
Copy link
Copy Markdown
Member

@elasticmachine test this please

@cbuescher cbuescher merged commit c713d62 into elastic:master Mar 16, 2018
cbuescher pushed a commit that referenced this pull request Mar 16, 2018
cbuescher pushed a commit that referenced this pull request Mar 16, 2018
cbuescher pushed a commit that referenced this pull request Mar 16, 2018
cbuescher pushed a commit that referenced this pull request Mar 16, 2018
cbuescher pushed a commit that referenced this pull request Mar 16, 2018
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/6.x: (89 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  [Docs] Fix link to Grok patterns (#29088)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  Fix Parsing Bug with Update By Query for Stored Scripts (#29039)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  [Docs] Fix Java Api index administration usage (#28260)
  Improve error message for installing plugin (#28298)
  Decouple XContentBuilder from BytesReference (#28972)
  ...
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/master: (97 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  Compilation fix for #29067
  [Docs] Fix link to Grok patterns (#29088)
  Store offsets in index prefix fields when stored in the parent field (#29067)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  CLI: Close subcommands in MultiCommand (#28954)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  Improve error message for installing plugin (#28298)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Ingest Node Execution or management of Ingest Pipelines >docs General docs changes v5.6.9 v6.0.3 v6.1.4 v6.2.3 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants