Skip to content

Remove useless files for TiFlash#1596

Merged
JaySon-Huang merged 9 commits intopingcap:masterfrom
JaySon-Huang:remove_useless
Mar 24, 2021
Merged

Remove useless files for TiFlash#1596
JaySon-Huang merged 9 commits intopingcap:masterfrom
JaySon-Huang:remove_useless

Conversation

@JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Mar 19, 2021 •
edited
Loading

What problem does this PR solve?

I want to make my editor index fewer files. There are many files not used by TiFlash.

What is changed and how it works?

Remove this submodule
contrib/capnproto

Remove these files

.gitlab-ci.yml
.travis.yml
CHANGELOG.md
CHANGELOG_RU.md
Jenkinsfile
MacOS.md
copy_headers.sh

debian/ -- tools for debian package
docker-compose.yml
docker/  -- dockefile for CK
website/ -- 
docs/     -- docs for CK
utils/      -- some utils for testing

release
release_lib.sh
dbms/benchmark/ -- benchmark with other dbms
dbms/scripts/ -- scripts  for generating test data

dbms/src/Server/clickhouse-client.xml
-- Some binaries that are useless for TiFlash
dbms/src/Server/ClusterCopier.cpp
dbms/src/Server/ClusterCopier.h
dbms/src/Server/clickhouse-copier.cpp
-- testing directories
dbms/src/Server/{data,metadata,users.d,config.d}/

Related changes

  • N/A

Check List

Tests

  • No code

Side effects

  • N/A

Release note

  • No release note

@JaySon-Huang JaySon-Huang self-assigned this Mar 19, 2021
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 19, 2021

/run-all-tests

1 similar comment
@tisonkun
Copy link
Contributor

tisonkun commented Mar 22, 2021

/run-all-tests

zanmato1984
zanmato1984 reviewed Mar 22, 2021
View reviewed changes
dbms/src/Server/CMakeLists.txt Outdated
Copy link
Contributor

@zanmato1984 zanmato1984 Mar 22, 2021

Choose a reason for hiding this comment

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

Can we keep this option and the relative libs/code? We may investigate using codegen someday.

@flowbehappy flowbehappy self-requested a review March 22, 2021 03:53
flowbehappy
flowbehappy reviewed Mar 22, 2021
View reviewed changes
dbms/src/Server/CMakeLists.txt Outdated
Copy link
Contributor

@flowbehappy flowbehappy Mar 22, 2021

Choose a reason for hiding this comment

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

Can we keep those "Benchmark.cpp, Compressor.cpp" files and by default do NOT compile them? I agree that we do not need them currently, but I think the idea here is very good, i.e. run benchmarks for each component separately.

Copy link
Contributor Author

@JaySon-Huang JaySon-Huang Mar 22, 2021

Choose a reason for hiding this comment

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

https://github.com/pingcap/tics/blob/6cee66f2a3ab93d909f83b7fef9a0ddbf894545b/dbms/src/Server/CMakeLists.txt#L6-L13
Yes, those binaries are not compiled if you run make tiflash

@JaySon-Huang
Clean useless files for TiFlash
aa3325a 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Restore some files under dbms/src/Server
199fb91 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Remove test directories and clickhouse-copier
667a89b 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Remove cluster copier
850d68c 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Clean
1597ef0 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 22, 2021

/run-all-tests

1 similar comment
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 22, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 22, 2021 •
edited
Loading

@zanmato1984 @flowbehappy I've restored those binary targets. PTAL again

@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 23, 2021

/run-all-tests

zanmato1984
zanmato1984 approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@zanmato1984 zanmato1984 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 Mar 24, 2021
@flowbehappy flowbehappy self-requested a review March 24, 2021 10:31
flowbehappy
flowbehappy reviewed Mar 24, 2021
View reviewed changes
Copy link
Contributor

@flowbehappy flowbehappy 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 Indicates that a PR has LGTM 1. label Mar 24, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Mar 24, 2021
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 24, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 24, 2021
@ti-srebot
Copy link
Collaborator

ti-srebot commented Mar 24, 2021

Your auto merge job has been accepted, waiting for:

  • 1621

@ti-srebot
Copy link
Collaborator

ti-srebot commented Mar 24, 2021

/run-all-tests

@ti-srebot
Copy link
Collaborator

ti-srebot commented Mar 24, 2021

@JaySon-Huang merge failed.

@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 24, 2021

/run-all-tests

1 similar comment
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 24, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 24, 2021

The test is running but shown as failure in Github. Waiting for it: https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/tics_ghpr_test/detail/tics_ghpr_test/4219/pipeline/19

@JaySon-Huang JaySon-Huang merged commit cef4247 into pingcap:master Mar 24, 2021
@JaySon-Huang JaySon-Huang deleted the remove_useless branch March 24, 2021 15:03
@JaySon-Huang JaySon-Huang mentioned this pull request Mar 25, 2021
Merged
This was referenced May 28, 2021
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flowbehappy flowbehappy flowbehappy left review comments

@zanmato1984 zanmato1984 zanmato1984 approved these changes

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

Assignees

@JaySon-Huang JaySon-Huang

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JaySon-Huang @tisonkun @ti-srebot @zanmato1984 @flowbehappy