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.

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

refactor: reorg workspace, a basic structure #7074

Merged
merged 22 commits into from Aug 13, 2022

Conversation

PsiACE
Copy link
Member

@PsiACE PsiACE commented Aug 10, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

In this PR

  • Splitting binary targets and shared libraries.
    • move fuzz to query/ast
    • move metabench, metactl to cmds
  • A clean root dir structure, all crates in src dir.
    • move databend-query to query/service
    • move databend-meta to meta/service
  • Almost all meta-related crates are categorised
    • move common/meta, common/protos, common/proto-conv to meta.
    • add a README.md to meta dir
  • edit makefile, codeowner and docs

Related #7050

@vercel
Copy link

vercel bot commented Aug 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Aug 13, 2022 at 3:04AM (UTC)

@mergify mergify bot added the pr-chore this PR only has small changes that no need to record, like coding styles. label Aug 10, 2022
@PsiACE PsiACE marked this pull request as draft August 10, 2022 16:06
@PsiACE PsiACE marked this pull request as ready for review August 10, 2022 17:18
@PsiACE PsiACE requested review from BohuTANG and Xuanwo August 10, 2022 17:22
@PsiACE PsiACE changed the title chore: reorg workspace (part 1) chore: reorg workspace, a basic structure Aug 10, 2022
@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

targets have different meanings in rust, and it seems we don't need to add an extra layer.

How about renaming cmds to bianries?

binaries
  - query
  - meta
  - metabench
  - metactl

@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

Rename common to components? Or we will do this in the next parts?

@PsiACE
Copy link
Member Author

PsiACE commented Aug 11, 2022

Rename common to components? Or we will do this in the next parts?

we will do this in the next parts

targets have different meanings in rust, and it seems we don't need to add an extra layer.

👍

How about renaming cmds to bianries?

cargo install databend-bianries ?

@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

cargo install databend-bianries ?

I guess we should only upload libs to crates.io. Binaries are so large that not suitable for users to cargo install them directly.

@BohuTANG
Copy link
Member

This is a monster PR!
Can we split into more than one PR? The layout change will block many other PRs, and other PRs will block this PR too.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Nice work!

@PsiACE
Copy link
Member Author

PsiACE commented Aug 11, 2022

I guess we should only upload libs to crates.io. Binaries are so large that not suitable for users to cargo install them directly.

It has now been renamed, but I still think it can be published to crates.io, which just means that users will need to compile it themselves.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

This is a monster PR! Can we split into more than one PR? The layout change will block many other PRs, and other PRs will block this PR too.

Most works are just renaming. git is smart enough to merge them without conflicts (I guess).

@PsiACE
Copy link
Member Author

PsiACE commented Aug 11, 2022

This is a monster PR! Can we split into more than one PR? The layout change will block many other PRs, and other PRs will block this PR too.

Most works are just renaming. git is smart enough to merge them without conflicts (I guess).

Yes, most of the time it is easy to avoid file conflicts by simply running git pull once.

As we plan to introduce layout changes, I think it will be difficult to avoid blocking each other.

@BohuTANG
Copy link
Member

Almost LGTM.
I suggest merging this PR on the weekend or when PR numbers are low, not blocking other PRs which is working.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 11, 2022

Ooops, there are many conflicts. Git is not smart enough 😢

@PsiACE PsiACE marked this pull request as draft August 11, 2022 02:02
@PsiACE
Copy link
Member Author

PsiACE commented Aug 11, 2022

I suggest merging this PR on the weekend or when PR numbers are low, not blocking other PRs which is working.

I marked it as a draft to avoid merging at an unexpected time.

Ooops, there are many conflicts. Git is not smart enough

emmm, It requires manual confirmation of the merge. But the fact is that hardly anything has to be modified.

@Xuanwo
Copy link
Member

Xuanwo commented Aug 12, 2022

This PR shows how frequently we change our code (just in one day)...

@Xuanwo
Copy link
Member

Xuanwo commented Aug 13, 2022

cc @PsiACE, Let's try merge this PR today!

@Xuanwo Xuanwo marked this pull request as ready for review August 13, 2022 02:57
@Xuanwo Xuanwo changed the title chore: reorg workspace, a basic structure refactor: reorg workspace, a basic structure Aug 13, 2022
@Xuanwo Xuanwo removed the pr-chore this PR only has small changes that no need to record, like coding styles. label Aug 13, 2022
@mergify mergify bot added the pr-refactor this PR changes the code base without new features or bugfix label Aug 13, 2022
@Xuanwo
Copy link
Member

Xuanwo commented Aug 13, 2022

Github is more stupid than Git.

git merge works excellent, but GitHub marks it as conflicts.

@mergify mergify bot merged commit 3d2f4bb into datafuselabs:main Aug 13, 2022
@PsiACE PsiACE deleted the reorg-workspace branch August 13, 2022 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants