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.

Sign up for GitHub

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

Jump to bottom

tidb-lightning: add glossary and other updates #1693

Merged
merged 18 commits into from
Feb 10, 2020
Merged

tidb-lightning: add glossary and other updates #1693

merged 18 commits into from
Feb 10, 2020

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Dec 7, 2019 •
edited
Loading

What is changed, added or deleted?

What is the related PR or file link(s)?

#1655 — I linked to a term ("region") from the glossary in that PR. That entry is copied into this PR to avoid having a broken link.

Which version does your change affect?

  • dev (glossary)
  • dev, v3.0, v3.1 (rest)

@kennytm kennytm added v3.0 translation/doing This PR's assignee is translating this PR. labels Dec 7, 2019
@kennytm kennytm force-pushed the kennytm/lightning-2019-12 branch from a7ab1f2 to 9b24c69 Compare December 11, 2019 21:30
@kennytm kennytm force-pushed the kennytm/lightning-2019-12 branch from 6d58944 to 305864c Compare December 13, 2019 01:31
@kennytm
Copy link
Contributor Author

kennytm commented Dec 13, 2019

PTAL @WangXiangUSTC

@IANTHEREAL
Copy link
Contributor

Cool! LGTM

@IANTHEREAL
Copy link
Contributor

@lilin90 @WangXiangUSTC PTAL

lilin90
lilin90 reviewed Dec 26, 2019
View reviewed changes
Copy link
Member

@lilin90 lilin90 left a comment

Choose a reason for hiding this comment

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

@dcalvin Could you please help review the language? Thanks!

@lilin90 lilin90 requested a review from dcalvin December 26, 2019 07:45
dcalvin
dcalvin reviewed Dec 26, 2019
View reviewed changes
Copy link
Contributor

@dcalvin dcalvin left a comment

Choose a reason for hiding this comment

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

See my edits in line.

dev/faq/tidb-lightning.md Outdated
@@ -28,11 +28,13 @@ TiDB Lightning requires the following privileges:
* CREATE
* DROP

If the target database is used to store checkpoints, it additionally requires these privileges:
If the ["TiDB" back end](/dev/reference/tools/tidb-lightning/tidb-backend.md) is chosen, or the target database is used to store checkpoints, it additionally requires these privileges:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove the quotation mark?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may feel strange in

TiDB Lightning TiDB back end

vs.

TiDB Lightning "TiDB" back end

Copy link
Contributor

Choose a reason for hiding this comment

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

Both of these two look weird to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestions 😂 The feature is called "back end", the choice is between tidb and importer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest removing it because TiDB is already a proper noun in a capitalized form. It can work alone without a quotation mark

Copy link
Contributor

@dcalvin dcalvin Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe TiDB-backend / importer-backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hyphenated in c3c7e2a.

dev/faq/tidb-lightning.md Outdated Show resolved Hide resolved
dev/faq/tidb-lightning.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/deployment.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/deployment.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
WangXiangUSTC
WangXiangUSTC reviewed Jan 1, 2020
View reviewed changes
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

dcalvin
dcalvin reviewed Jan 2, 2020
View reviewed changes
dev/faq/tidb-lightning.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/glossary.md Outdated Show resolved Hide resolved
dev/reference/tools/tidb-lightning/overview.md Outdated Show resolved Hide resolved
@kennytm
Copy link
Contributor Author

kennytm commented Jan 9, 2020

PTAL again @dcalvin thanks

@kennytm
Copy link
Contributor Author

kennytm commented Jan 14, 2020

Ping @dcalvin

@anotherrachel anotherrachel self-assigned this Jan 14, 2020
@anotherrachel anotherrachel mentioned this pull request Jan 14, 2020
Merged
@dcalvin
Copy link
Contributor

dcalvin commented Feb 3, 2020

Sorry for the much delayed response. Plz address the most recent comments. Otherwise LGTM @kennytm

@kennytm
Copy link
Contributor Author

kennytm commented Feb 3, 2020

@dcalvin @anotherrachel Addressed comments in e36e020.

dcalvin
dcalvin approved these changes Feb 3, 2020
View reviewed changes
Copy link
Contributor

@dcalvin dcalvin left a comment

Choose a reason for hiding this comment

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

LGTM

@anotherrachel anotherrachel added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. requires-followup This PR requires a follow-up task after being merged. and removed translation/doing This PR's assignee is translating this PR. labels Feb 4, 2020
@anotherrachel anotherrachel mentioned this pull request Feb 6, 2020
Merged
12 tasks
@anotherrachel anotherrachel removed the requires-followup This PR requires a follow-up task after being merged. label Feb 6, 2020
@yikeke yikeke added the status/can-merge Indicates a PR has been approved by a committer. label Feb 10, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 10, 2020

/run-all-tests

@sre-bot sre-bot merged commit 20f45a2 into master Feb 10, 2020
@kennytm kennytm deleted the kennytm/lightning-2019-12 branch February 10, 2020 15:57
yikeke pushed a commit to sre-bot/docs that referenced this pull request Mar 24, 2020
add lightning gloassry to v3.0 (ref: pingcap#1693)
8abcc3e
yikeke pushed a commit to sre-bot/docs that referenced this pull request Mar 24, 2020
add lightning gloassry to v3.0 (ref: pingcap#1693)
c9b5292
yikeke pushed a commit that referenced this pull request Mar 24, 2020
@sre-bot @toutdesuite
tools: add TOC for sync-diff-inspector (#2077) (#2083)
f58e783 
* cherry pick #2077 to release-3.1

Signed-off-by: sre-bot <sre-bot@pingcap.com>

* add lightning gloassry to v3.0 (ref: #1693)

* resolve conflict

Co-authored-by: toutdesuite <guizhiluo2014@163.com>
Co-authored-by: yikeke <yikeke@pingcap.com>
yikeke pushed a commit that referenced this pull request Mar 24, 2020
@sre-bot @toutdesuite
tools: add TOC for sync-diff-inspector (#2077) (#2082)
def55c1 
* cherry pick #2077 to release-3.0

Signed-off-by: sre-bot <sre-bot@pingcap.com>

* add lightning gloassry to v3.0 (ref: #1693)

* resolve conflict

Co-authored-by: toutdesuite <guizhiluo2014@163.com>
Co-authored-by: yikeke <yikeke@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anotherrachel anotherrachel anotherrachel left review comments

@lilin90 lilin90 lilin90 left review comments

@IANTHEREAL IANTHEREAL IANTHEREAL left review comments

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

@csuzhangxc csuzhangxc csuzhangxc left review comments

@dcalvin dcalvin dcalvin approved these changes

Assignees

@anotherrachel anotherrachel

Labels
status/can-merge Indicates a PR has been approved by a committer. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@kennytm @IANTHEREAL @dcalvin @sre-bot @csuzhangxc @WangXiangUSTC @lilin90 @anotherrachel @yikeke