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

Edits to text in Update API doc #39069

Merged
merged 2 commits into from
Feb 20, 2019
Merged

Conversation

dmeiss
Copy link
Contributor

@dmeiss dmeiss commented Feb 18, 2019

Minor edits to text for formatting, punctuation, and word usage.

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

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

To me the changes is fine as is but I left one comment / suggestion.

@danielmitterdorfer danielmitterdorfer added :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v7.0.0 v6.7.0 v8.0.0 v7.2.0 v6.6.2 labels Feb 19, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@danielmitterdorfer
Copy link
Member

@elasticmachine test this please

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dmeiss
Copy link
Contributor Author

dmeiss commented Feb 19, 2019

Hi Daniel:

General question: I'm a longtime technical editor, but I'm pretty new at using GitHub -- is there any way I should be going about this process differently? i.e., is there anything I can do that makes this easier on reviewers? I just want to make sure I'm following the rules, best practices, etc.

Thx,
D

@danielmitterdorfer
Copy link
Member

I think your approach is totally fine. We prefer small, targeted PRs to massive ones because that makes it easier to manage (e.g. a PR that is "fixing everything that's wrong in your docs" is probably much harder to get in than one that covers specific aspects).

Personally, I find your PRs rather good and quick to review so I think you should just stick to your current approach. As you may inferred from the number of version labels that I've applied to your PRs, we need to port them to quite a few branches though because our documentation is versioned but I'd still prefer this to massive PRs which would then cause a lot of merge conflicts anyway.

At this point I also want to thank you for your contributions; that's really helpful and an immediate improvement for our entire user base.

@dmeiss
Copy link
Contributor Author

dmeiss commented Feb 19, 2019

Thanks much for the feedback. I'll keep small PRs in mind when editing some of those really long docs. I'll try to edit just a few sections at a time to keep the PR more concise.

@dmeiss
Copy link
Contributor Author

dmeiss commented Feb 20, 2019

@danielmitterdorfer
Hi Daniel:

I have another general question, if you don't mind. (I'm also not sure where to bring this up, so I just tacked this comment onto this PR.)

While editing docs, I've noticed that the link tooltips for anchor links append the word "edit" to the tooltip text, e.g. the link "Search Cancellation" has a tooltip of "Search Cancellationedit". In looking at the HTML, I can see that it's because each heading also contains the Edit icon and text, and it appears that the anchor link's title attribute is formed based off all the text in the heading tag:

<a class="xref" href="search.html#global-search-cancellation" title="Search Cancellationedit">Search Cancellation<a href="https://github.com/elastic/elasticsearch/edit/6.6/docs/reference/search.asciidoc" class="edit_me" title="Edit this page on GitHub" rel="nofollow">edit</a></a>
<h2><a id="global-search-cancellation"></a>Search Cancellation<a href="https://github.com/elastic/elasticsearch/edit/6.6/docs/reference/search.asciidoc" class="edit_me" title="Edit this page on GitHub" rel="nofollow">edit</a></h2>

It's certainly not the biggest deal in the world, but ideally the link tooltips would look much better without it.

@danielmitterdorfer
Copy link
Member

@dmeiss thanks for the feedback. I think it's best to raise an issue so our docs team can have a look at this. Can you please create one?

@dmeiss
Copy link
Contributor Author

dmeiss commented Feb 27, 2019

@danielmitterdorfer Hi Daniel -- thanks for the response. I also posted this on the Elastic forum. Nik Everett opened an issue for it.

@danielmitterdorfer
Copy link
Member

That's great, thanks for following up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >docs General docs changes v6.6.2 v6.7.0 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants