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

pkg/lightning:move mock file for lightning into lightning package#837

Closed
AndreMouche wants to merge 1 commit into
masterfrom
lightning/mock
Closed

pkg/lightning:move mock file for lightning into lightning package#837
AndreMouche wants to merge 1 commit into
masterfrom
lightning/mock

Conversation

@AndreMouche
Copy link
Copy Markdown
Contributor

@AndreMouche AndreMouche commented Mar 10, 2021

Signed-off-by: shirly AndreMouche@126.com

What problem does this PR solve?

move the mock files for lightning into the lightning package, since these files are only used in tests for lightning.
Meanwhile after this PR merged, tidb needn't import github.com/cockroachdb/pebble indirectly.

related PR:#786

Check List

Tests

  • Unit test
  • Integration test

Release Note

  • No release note

Signed-off-by: shirly <AndreMouche@126.com>
@ti-srebot
Copy link
Copy Markdown
Contributor

@disksing, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: migrate(slack).

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.

Please do not split mock package.

@AndreMouche
Copy link
Copy Markdown
Contributor Author

why? I think it is only used for lighting, so we could and should split it. If we should merge them together, so why we need to have an independent package for lighting?

Please do not split mock package.

why? I think it is only used for lighting, so we could and should split it. If we should merge them together, so why we need to have an independent package for lighting?

@kennytm
Copy link
Copy Markdown
Collaborator

kennytm commented Mar 10, 2021

This PR is caused by pingcap/tidb#23183.

The issue is like this:

  1. After merging Lightning into BR, BR as a module now depends on Pebble.
  2. Pebble depends on github.com/cockroachdb/errors
  3. github.com/cockroachdb/errors depends on Protobuf 1.4.2
  4. But TiDB currently requires Protobuf at =1.3.4 due to some maintenance issue or something.

Current solution in 23183 is to do replace protobuf => protobuf 1.3.4. Now trying an alternate solution of making the mocks test-only.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants