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

Refine .clang-format #2791

Merged
merged 1 commit into from
Jan 17, 2020
Merged

Refine .clang-format #2791

merged 1 commit into from
Jan 17, 2020

Conversation

gaodayue
Copy link
Contributor

Part of #2790

The goal is to

  1. Choose the most popular style options based on existing code, in order to minimize reformatting diffs
  2. Remove options that are same with Google, to make it clear the differences between Doris's style and Google's

@gaodayue
Copy link
Contributor Author

FYI @vagetablechicken

@vagetablechicken
Copy link
Contributor

is it ok with SortIncludes diffs? It's true in Google style.

@gaodayue
Copy link
Contributor Author

gaodayue commented Jan 17, 2020

is it ok with SortIncludes diffs? It's true in Google style.

Hi @vagetablechicken , thanks for the reply. I've tried using this file to reformat existing code, it turns out that the majority of diffs are not caused by SortIncludes, but ColumnLimit instead.

Clang-format will try to put as many arguments as possible into the the same line if ColumnLimit is not reached or insert breaks otherwise. For example

-    _create_tablet_workers = new TaskWorkerPool(
-            TaskWorkerPool::TaskWorkerType::CREATE_TABLE,
-            _exec_env,
-            master_info);
-    _drop_tablet_workers = new TaskWorkerPool(
-            TaskWorkerPool::TaskWorkerType::DROP_TABLE,
-            _exec_env,
-            master_info);
-    _push_workers = new TaskWorkerPool(
-            TaskWorkerPool::TaskWorkerType::PUSH,
-            _exec_env,
-            master_info);
-    _publish_version_workers = new TaskWorkerPool(
-            TaskWorkerPool::TaskWorkerType::PUBLISH_VERSION,
-            _exec_env,
-            master_info);
+    _create_tablet_workers = new TaskWorkerPool(TaskWorkerPool::TaskWorkerType::CREATE_TABLE,
+                                                _exec_env, master_info);
+    _drop_tablet_workers =
+            new TaskWorkerPool(TaskWorkerPool::TaskWorkerType::DROP_TABLE, _exec_env, master_info);
+    _push_workers =
+            new TaskWorkerPool(TaskWorkerPool::TaskWorkerType::PUSH, _exec_env, master_info);
+    _publish_version_workers = new TaskWorkerPool(TaskWorkerPool::TaskWorkerType::PUBLISH_VERSION,
+                                                  _exec_env, master_info);

Another major diff is caused by AlignAfterOpenBracket. For example with AlignAfterOpenBracket set to Align

-void TaskWorkerPool::_alter_tablet(
-        TaskWorkerPool* worker_pool_this,
-        const TAgentTaskRequest& agent_task_req,
-        int64_t signature,
-        const TTaskType::type task_type,
-        TFinishTaskRequest* finish_task_request) {
+void TaskWorkerPool::_alter_tablet(TaskWorkerPool* worker_pool_this,
+                                   const TAgentTaskRequest& agent_task_req, int64_t signature,
+                                   const TTaskType::type task_type,
+                                   TFinishTaskRequest* finish_task_request) {

However I can't find a way to allow multiple align styles. Seems that one style must be chosen and Align is widely used and looks good to me.

@vagetablechicken
Copy link
Contributor

That's nice, I prefer sorting includes.

Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

LGTM

@imay imay merged commit a3789ab into apache:master Jan 17, 2020
songenjie pushed a commit to songenjie/incubator-doris that referenced this pull request Apr 8, 2020
The problem of inconsistence style in Doris code is too big, it's hard to minimize modification when reformatting code.
So here, our aim is to make the style rules, tune the config in .clang-format.

Note: I choose clang-format-8.0+ to support richer sytle options.

2. Refine .clang-format (apache#2791)
songenjie pushed a commit to songenjie/incubator-doris that referenced this pull request Apr 8, 2020
- Add .clang-format and docs (apache#2724)

The problem of inconsistence style in Doris code is too big, it's hard to minimize modification when reformatting code.
So here, our aim is to make the style rules, tune the config in .clang-format.

Note: I choose clang-format-8.0+ to support richer sytle options.

- Refine .clang-format (apache#2791)
songenjie pushed a commit to songenjie/incubator-doris that referenced this pull request Apr 8, 2020
- Add .clang-format and docs (apache#2724)

The problem of inconsistence style in Doris code is too big, it's hard to minimize modification when reformatting code.
So here, our aim is to make the style rules, tune the config in .clang-format.

Note: I choose clang-format-8.0+ to support richer sytle options.

- Refine .clang-format (apache#2791)
marising pushed a commit to marising/incubator-doris that referenced this pull request Jun 1, 2020
- Add .clang-format and docs (apache#2724)

The problem of inconsistence style in Doris code is too big, it's hard to minimize modification when reformatting code.
So here, our aim is to make the style rules, tune the config in .clang-format.

Note: I choose clang-format-8.0+ to support richer sytle options.

- Refine .clang-format (apache#2791)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants