Skip to content

Merge developer guide formatting issues#15577

Merged
electrum merged 4 commits intotrinodb:masterfrom
nineinchnick:merge-docs-errors
Feb 9, 2023
Merged

Merge developer guide formatting issues#15577
electrum merged 4 commits intotrinodb:masterfrom
nineinchnick:merge-docs-errors

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

The merge developer guide has some formatting issues and some mismatches with the API. This PR fixes them and applies the Google style guide, checked using the Vale linter.

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Jan 3, 2023
@nineinchnick
Copy link
Copy Markdown
Member Author

@djsstarburst @electrum please review, especially the first two commits. @mosabua please review the last two commits (style).

@github-actions github-actions bot added the docs label Jan 3, 2023
Copy link
Copy Markdown
Member

@djsagain djsagain left a comment

Choose a reason for hiding this comment

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

Thanks for doing this; the changes look fine.

I was under the impression that two spaces were required between a period and the start of the next sentence, so the change to make it a single space surprises me.

@nineinchnick
Copy link
Copy Markdown
Member Author

I was under the impression that two spaces were required between a period and the start of the next sentence, so the change to make it a single space surprises me.

Yeah, it was weirdly consistent 😄 I don't think it's required though, other docs don't use it.

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Nice! This could be merged as is or with the commits collapsed .. either works for me.

@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jan 3, 2023

Thanks for doing this; the changes look fine.

I was under the impression that two spaces were required between a period and the start of the next sentence, so the change to make it a single space surprises me.

The two spaces are NOT required. The rendering pipeline for the output collapsed them into one. The usage of two spaces dates back to typewriter and fixed-width fonts only usage, don't do that anymore these days please..

Copy link
Copy Markdown
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

Generally looks good. Thanks for improving this.

@electrum electrum merged commit 7a905ff into trinodb:master Feb 9, 2023
@github-actions github-actions bot added this to the 407 milestone Feb 9, 2023
@nineinchnick nineinchnick deleted the merge-docs-errors branch May 27, 2024 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants