Skip to content

Add shard-row-id-bits.md doc and fix some links#3239

Merged
yikeke merged 9 commits intomasterfrom
add-shard-row-id-bits
Jul 14, 2020
Merged

Add shard-row-id-bits.md doc and fix some links#3239
yikeke merged 9 commits intomasterfrom
add-shard-row-id-bits

Conversation

@yikeke
Copy link
Contributor

@yikeke yikeke commented Jul 10, 2020 •
edited
Loading

What is changed, added or deleted? (Required)

Which TiDB version(s) do your changes apply to? (Required)

  • master (the latest development version)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

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

@github-actions github-actions bot requested review from a user, TomShawn, bb7133, kissmydb, lilin90 and zimulala July 10, 2020 15:06
@github-actions github-actions bot added translation/welcome Waits for a contributor to translate this PR and create a PR to the pingcap/docs-cn repository. status/PTAL This PR is ready for reviewing. labels Jul 10, 2020
@yikeke yikeke added needs-cherry-pick-4.0 translation/from-docs-cn This PR is translated from a PR in pingcap/docs-cn. and removed translation/welcome Waits for a contributor to translate this PR and create a PR to the pingcap/docs-cn repository. labels Jul 10, 2020
@ghost
Copy link

ghost commented Jul 10, 2020

This will conflict with #3152 - can we merge 3152 first, and then this one?

@yikeke yikeke mentioned this pull request Jul 10, 2020
Merged
5 tasks
@yikeke
Copy link
Contributor Author

yikeke commented Jul 10, 2020 •
edited
Loading

This will conflict with #3152 - can we merge 3152 first, and then this one?

This PR is only relocating the content to a new file and can be merged quickly. I suppose #3152 would have a longer reviewing process, so I prefer merging this PR first? Is that okay? @nullnotnil

And this PR documents the shard-row-id-bits part on a separate page so you can just link it in #3152. I believe that might simplify your changes in #3152 a bit?

@ghost
Copy link

ghost commented Jul 10, 2020

This PR is only relocating the content to a new file and can be merged quickly. I suppose #3152 would have a longer reviewing process, so I prefer merging this PR first? Is that okay? @nullnotnil

It is ready to go. It is large in files it touches, but I don't see why review should take long. I have already merged conflicts from another PR that came later (#3215), if I keep having to do that it is going to discourage making improvements to documents.

@ti-srebot
Copy link
Contributor

ti-srebot commented Jul 12, 2020

@kissmydb, @nullnotnil, PTAL.

@yikeke yikeke requested a review from jackysp July 13, 2020 01:58
@yikeke
Copy link
Contributor Author

yikeke commented Jul 13, 2020

This PR is only relocating the content to a new file and can be merged quickly. I suppose #3152 would have a longer reviewing process, so I prefer merging this PR first? Is that okay? @nullnotnil

It is ready to go. It is large in files it touches, but I don't see why review should take long. I have already merged conflicts from another PR that came later (#3215), if I keep having to do that it is going to discourage making improvements to documents.

Actually I was gonna add my commits in this PR to #3215 to fix the deadlink check back then. But something else delayed me for a few hours and when I realized #3215 was merged😂 with one LGTM.

Please note that according to the new docs collaboration process, we should try to merge every PR after it gets at least two LGTMs - one from the R&D side and one from the language/format side (some PRs should also get an LGTM from DBA's side). We want to take everything a bit slower so we can make things right once. Does this make sense to you? @nullnotnil

As regard to merging #3239 or #3152 first, it's both okay for me (the conflict will be easy to resolve for both PRs). Just a note from my experience, larger changes take longer to get two or three LGTMs.

kissmydb
kissmydb approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@kissmydb kissmydb left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 13, 2020
@ti-srebot
Copy link
Contributor

ti-srebot commented Jul 13, 2020

@kissmydb,Thanks for your review.

@yikeke yikeke requested a review from ran-huang July 13, 2020 03:35
@yikeke yikeke mentioned this pull request Jul 13, 2020
Merged
5 tasks
ran-huang
ran-huang reviewed Jul 13, 2020
View reviewed changes
@yikeke yikeke mentioned this pull request Jul 14, 2020
Merged
5 tasks
yikeke
yikeke commented Jul 14, 2020
View reviewed changes
yikeke
yikeke commented Jul 14, 2020
View reviewed changes
@yikeke yikeke added the requires-version-specific-changes After cherry-picked, the cherry-picked PR requires further changes. label Jul 14, 2020
@yikeke
Copy link
Contributor Author

yikeke commented Jul 14, 2020

I‘ve resolved all the conflicts caused by #3152. This is ready for review. PTAL @nullnotnil

@yikeke yikeke added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 14, 2020
ghost
ghost reviewed Jul 14, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

ti-srebot commented Jul 14, 2020

@nullnotnil,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: docs(slack).

@yikeke yikeke merged commit 2bc391f into master Jul 14, 2020
ti-srebot pushed a commit to ti-srebot/docs that referenced this pull request Jul 14, 2020
@yikeke @ti-srebot
cherry pick pingcap#3239 to release-4.0
b36ded9 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jul 14, 2020
Merged
5 tasks
@ti-srebot
Copy link
Contributor

ti-srebot commented Jul 14, 2020

cherry pick to release-4.0 in PR #3267

ti-srebot added a commit that referenced this pull request Jul 15, 2020
@ti-srebot
Add shard-row-id-bits.md doc and fix some links (#3239) (#3267)
2801849 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@lilin90 lilin90 deleted the add-shard-row-id-bits branch August 5, 2020 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ti-srebot ti-srebot ti-srebot approved these changes

@jackysp jackysp Awaiting requested review from jackysp

+3 more reviewers

@ran-huang ran-huang ran-huang left review comments

@kissmydb kissmydb kissmydb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

requires-version-specific-changes After cherry-picked, the cherry-picked PR requires further changes. status/LGT2 Indicates that a PR has LGTM 2. status/PTAL This PR is ready for reviewing. translation/from-docs-cn This PR is translated from a PR in pingcap/docs-cn.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yikeke @ti-srebot @kissmydb @ran-huang