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 missing example script quote #32010

Merged
merged 1 commit into from
Jul 17, 2018
Merged

Docs: Fix missing example script quote #32010

merged 1 commit into from
Jul 17, 2018

Conversation

aptxx
Copy link
Contributor

@aptxx aptxx commented Jul 12, 2018

it seems lost right quote

Copy link
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.

@aptxx thanks for noting this, look right to me. Ideally this should have been caught by the test. I'm currently looking into why this is not causing an error. Maybe it makes sense to add an assertion to the response that at least checks there are no failures. I will check if this would have detected this. Maybe you can add it to this PR in that case.

@colings86 colings86 added >docs General docs changes :Data Management/Indices APIs APIs to create and manage indices and templates labels Jul 13, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@cbuescher
Copy link
Member

@aptxx this is definitely an error which we should correct. Would you mind signing the CLA so I can merge this change?

@cbuescher cbuescher self-assigned this Jul 16, 2018
@aptxx
Copy link
Contributor Author

aptxx commented Jul 16, 2018

@cbuescher Done. I found there are many test case without exception handling in the file. Did them run as expected? I will have a little check if it's need

@cbuescher
Copy link
Member

@aptxx Yes, the problem with ReindexDocumentationIT seems to be that none of the test methods is actually run at the moment. We rely on test methods starting with "test..." in out test infrastructure, so I think none of them are run at the moment (which might be intentional, the Documentation tests are mainly there to include code snippets from and should mostly allow us catching compile errors in the java syntax). I think we should run those tests but I doubt this would work at the moment. I'd like to merge this change and open a separate issue about improving ReindexDocumentationIT so it actually runs the code and catches things like this.

@cbuescher
Copy link
Member

@elasticmachine test this please

Copy link
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.

LGTM

@cbuescher cbuescher changed the title docs: fix example quote missing Docs: Fix missing example script quote Jul 16, 2018
@cbuescher
Copy link
Member

@aptxx thanks again for catching this, I will merge this change as soon as CI passes.
I opened #32093 to improve this test suite, in case you are interested in picking it up. It might mean some changes to the test setup for this class though, I haven't checked what kind of work is required.

@aptxx
Copy link
Contributor Author

aptxx commented Jul 16, 2018

@cbuescher ok. I will handle that issue later

@cbuescher
Copy link
Member

@aptxx there are test failures unrelated to your documentation change, could you either merge in or rebase this PR on master please? I was about to do it on your PR branch but since it is also "master" in your forked repo, I didn't want to push to it...

@aptxx
Copy link
Contributor Author

aptxx commented Jul 16, 2018

@cbuescher oh... sorry for my mistake. I will make another PR with named branch, It's ok?

@cbuescher
Copy link
Member

Sure, but you don't have to open a new PR I think. If you rebase your commit (aptxx:master) on on the current ES master branch then it also should be fine. You can reset your repos master after the merge.

@cbuescher
Copy link
Member

@elasticmachine test this please

@cbuescher
Copy link
Member

@aptxx I merged this to master and all the 6.x branches, thanks again for fixing.

dnhatn added a commit that referenced this pull request Jul 19, 2018
* 6.x:
  Fix rollup on date fields that don't support epoch_millis (#31890)
  Revert "Introduce a Hashing Processor (#31087)" (#32179)
  [test] use randomized runner in packaging tests (#32109)
  Painless: Fix caching bug and clean up addPainlessClass. (#32142)
  Fix BwC Tests looking for UUID Pre 6.4 (#32158) (#32169)
  Call setReferences() on custom referring tokenfilters in _analyze (#32157)
  Add more contexts to painless execute api (#30511)
  Add EC2 credential test for repository-s3 (#31918)
  Fix CP for namingConventions when gradle home has spaces (#31914)
  Convert Version to Java - clusterformation part1 (#32009)
  Fix Java 11 javadoc compile problem
  Improve docs for search preferences (#32098)
  Configurable password hashing algorithm/cost(#31234) (#32092)
  [DOCS] Update TLS on Docker for 6.3
  ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are
  Switch distribution to new style Requests (#30595)
  Build: Skip jar tests if jar disabled
  Build: Move shadow customizations into common code (#32014)
  Painless: Add PainlessClassBuilder (#32141)
  Fix accidental duplication of bwc test for script behavior
  Handle missing values in painless (#30975) (#31903)
  Build: Make additional test deps of check (#32015)
  Painless: Fix Bug with Duplicate PainlessClasses (#32110)
  Adjust translog after versionType removed in 7.0 (#32020)
  Disable C2 from using AVX-512 on JDK 10 (#32138)
  [Rollup] Add new capabilities endpoint for concrete rollup indices (#32111)
  Mute :qa:mixed-cluster indices.stats/10_index/Index - all’
  [ML] Wait for aliases in multi-node tests (#32086)
  Ensure to release translog snapshot in primary-replica resync (#32045)
  Docs: Fix missing example script quote (#32010)
  Add Index UUID to `/_stats` Response (#31871) (#32113)
  [ML] Move analyzer dependencies out of categorization config (#32123)
  [ML][DOCS] Add missing 6.3.0 release notes (#32099)
  Updates the build to gradle 4.9 (#32087)
  Update monitoring template version to 6040099 (#32088)
  Fix put mappings java API documentation (#31955)
  Add exclusion option to `keep_types` token filter (#32012)
dnhatn added a commit that referenced this pull request Jul 20, 2018
* master:
  Painless: Simplify Naming in Lookup Package (#32177)
  Handle missing values in painless (#32207)
  add support for write index resolution when creating/updating documents (#31520)
  ECS Task IAM profile credentials ignored in repository-s3 plugin (#31864)
  Remove indication of future multi-homing support (#32187)
  Rest test - allow for snapshots to take 0 milliseconds
  Make x-pack-core generate a pom file
  Rest HL client: Add put watch action (#32026)
  Build: Remove pom generation for plugin zip files (#32180)
  Fix comments causing errors with Java 11
  Fix rollup on date fields that don't support epoch_millis (#31890)
  Detect and prevent configuration that triggers a Gradle bug (#31912)
  [test] port linux package packaging tests (#31943)
  Revert "Introduce a Hashing Processor (#31087)" (#32178)
  Remove empty @return from JavaDoc
  Adjust SSLDriver behavior for JDK11 changes (#32145)
  [test] use randomized runner in packaging tests (#32109)
  Add support for field aliases. (#32172)
  Painless: Fix caching bug and clean up addPainlessClass. (#32142)
  Call setReferences() on custom referring tokenfilters in _analyze (#32157)
  Fix BwC Tests looking for UUID Pre 6.4 (#32158)
  Improve docs for search preferences (#32159)
  use before instead of onOrBefore
  Add more contexts to painless execute api (#30511)
  Add EC2 credential test for repository-s3 (#31918)
  A replica can be promoted and started in one cluster state update (#32042)
  Fix Java 11 javadoc compile problem
  Fix CP for namingConventions when gradle home has spaces (#31914)
  Fix `range` queries on `_type` field for singe type indices (#31756)
  [DOCS] Update TLS on Docker for 6.3 (#32114)
  ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are
  Remove versionType from translog (#31945)
  Switch distribution to new style Requests (#30595)
  Build: Skip jar tests if jar disabled
  Painless: Add PainlessClassBuilder (#32141)
  Build: Make additional test deps of check (#32015)
  Disable C2 from using AVX-512 on JDK 10 (#32138)
  Build: Move shadow customizations into common code (#32014)
  Painless: Fix Bug with Duplicate PainlessClasses (#32110)
  Remove empty @param from Javadoc
  Re-disable packaging tests on suse boxes
  Docs: Fix missing example script quote (#32010)
  [ML] Wait for aliases in multi-node tests (#32086)
  [ML] Move analyzer dependencies out of categorization config (#32123)
  Ensure to release translog snapshot in primary-replica resync (#32045)
  Handle TokenizerFactory  TODOs (#32063)
  Relax TermVectors API to work with textual fields other than TextFieldType (#31915)
  Updates the build to gradle 4.9 (#32087)
  Mute :qa:mixed-cluster indices.stats/10_index/Index - all’
  Check that client methods match API defined in the REST spec (#31825)
  Enable testing in FIPS140 JVM (#31666)
  Fix put mappings java API documentation (#31955)
  Add exclusion option to `keep_types` token filter (#32012)
  [Test] Modify assert statement for ssl handshake (#32072)
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >docs General docs changes v6.0.3 v6.1.5 v6.2.5 v6.3.2 v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants