-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Spelling docs #37046
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
Spelling docs #37046
Conversation
1de0b5d to
c2336bb
Compare
|
Changes from the original PR:
|
c2336bb to
2cadc05
Compare
|
Pinging @elastic/es-docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nik9000 is there anything we can do do make this type of typo fail the docs build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, I've written Travis integration for checkstyle: https://github.com/checkstyle/checkstyle/blob/master/.ci/test-spelling-unknown-words.sh
The same sort of thing can be done in any project. If you're not a fan of shell scripts, you can of course port the code to Java/Groovy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking the speeling would work, but in this specific case, given that :request: and :response: as well as :api: are some kind of keyword needed to fill the docs content, we could fail the build when anything other than the known words are used within those delimiters. I think :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nik9000 another one that we may want to catch
...org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachmentParser.java
Outdated
Show resolved
Hide resolved
...lasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachmentParserTests.java
Outdated
Show resolved
Hide resolved
|
test this please |
|
retest this please |
|
I took the liberty to merge master in, as there were a couple of noisy tests causing failures that got muted on master meanwhile. Remember to pull from your own remote ;) |
|
I've rebased forward and cleaned out the files that were supposed to be renamed (my original hg repo properly had renames, but somewhere along the way the delete got lost) |
|
retest this please |
|
Seems like the build is failing due to some leftover references to the classes being renamed: |
|
Yeah, those commits aren't supposed to be there, I'm trying to figure out what I did wrong... |
| new ParseField("compare_analyzers"), CompareAnalyzers::parse)); | ||
| NamedXContentRegistry executeableSectionRegistry = new NamedXContentRegistry(entries); | ||
| return ESClientYamlSuiteTestCase.createParameters(executeableSectionRegistry); | ||
| NamedXContentRegistry executableSectionRegistry = new NamedXContentRegistry(entries); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now the only file that's a .java -- it's a bit odd that it lives in docs/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, I think it's there because it's the test class that tests the docs snippets. I think it makes sense for it to be there. This way when running ./gradlew :docs:check that one gets run.
|
@javanna : hopefully this should pass |
|
retest this please |
|
run gradle build tests 2 |
|
retest this please |
| * Added <<actions-pagerduty, PagerDuty action>> | ||
| * Added support for adding <<configuring-email-attachments, attachments to emails>> | ||
| via HTTP requests and superceding and deprecating the usage of `attach_data` | ||
| via HTTP requests and superseding and deprecating the usage of `attach_data` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't consider this one wrong, but otherwise the changes LGTM
|
run gradle build tests 2 |
|
run gradle build tests 1 |
|
retest this please |
|
thanks a lot @jsoref for all these fixes !!! And even more for making us fix the 5 different ways we were able to misspell the name of our own product :) |
|
Fwiw, that isn't uncommon :-o |
This is the docs portion of #37035 split per @tvernum.
Since there were so few
i.e.s here, I just left them, it isn't worth dropping, and for docs it seems more useful to leave them fixed.There are probably merge conflicts, but realistically, as noted in the other PR, there will always be merge conflicts, this repo moves way too fast for large changes like this. I'll cross that bridge closer to when reviews are done.
--
Generated by https://github.com/jsoref/spelling
f; to maintain your repo, please considerfchurn