Skip to content

Refine .clang-format - #2791

Merged
imay merged 1 commit into
apache:masterfrom
gaodayue:refine-clang-format
Jan 17, 2020
Merged

Refine .clang-format#2791
imay merged 1 commit into
apache:masterfrom
gaodayue:refine-clang-format

Conversation

@gaodayue

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

FYI @vagetablechicken

@vagetablechicken

Copy link
Copy Markdown
Member

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

@gaodayue

gaodayue commented Jan 17, 2020

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Member

That's nice, I prefer sorting includes.

@imay imay left a comment

Copy link
Copy Markdown
Contributor

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