Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

*: move lightning code into br#512

Merged
3pointer merged 6 commits into
pingcap:masterfrom
3pointer:lightning_code
Sep 18, 2020
Merged

*: move lightning code into br#512
3pointer merged 6 commits into
pingcap:masterfrom
3pointer:lightning_code

Conversation

@3pointer
Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Split #482

What is changed and how it works?

move lightning code into br.

Check List

Tests

  • No code

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • No release not

Comment thread pkg/decoder.go Outdated
// See the License for the specific language governing permissions and
// limitations under the License.

package decoder
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this file be placed in pkg/decoder/decoder.go or perhaps better pkg/cdclog/decoder.go

Copy link
Copy Markdown
Collaborator Author

@3pointer 3pointer Sep 18, 2020

Choose a reason for hiding this comment

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

I prefer put it in pkg/cdclog/decoder.go @overvenus #482 (comment) WDYT?

Comment thread pkg/restore/bytes.go Outdated
Comment thread pkg/restore/cdclog/checksum.go Outdated
@3pointer
Copy link
Copy Markdown
Collaborator Author

/run-integration-test

@3pointer 3pointer requested a review from overvenus September 18, 2020 06:40
Copy link
Copy Markdown
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Rest LGTM

// See the License for the specific language governing permissions and
// limitations under the License.

package cdclog
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you move the package to pkg too?

Copy link
Copy Markdown
Member

@overvenus overvenus 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 LGTM1 label Sep 18, 2020
Copy link
Copy Markdown
Collaborator

@kennytm kennytm 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 removed the status/LGT1 LGTM1 label Sep 18, 2020
@ti-srebot ti-srebot added the status/LGT2 LGTM2 label Sep 18, 2020
@3pointer
Copy link
Copy Markdown
Collaborator Author

/run-integration-test

@ti-srebot
Copy link
Copy Markdown
Contributor

cherry pick to release-4.0 in PR #529

overvenus pushed a commit that referenced this pull request Sep 29, 2020
Co-authored-by: luancheng <luancheng@pingcap.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants