You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/01/17 09:44:08 UTC
[GitHub] [incubator-doris] gaodayue opened a new pull request #2791: Refine
.clang-format
gaodayue opened a new pull request #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on issue #2791:
Refine .clang-format
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575564300
In Google style, `ColumnLimit: 80`.
So each line of text in code should be at most 80 characters long. It will generate many reformatting diffs. Is it OK？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575566915
is it ok with `SortIncludes` diffs? It's true in Google style.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] imay merged pull request #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
imay merged pull request #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] gaodayue commented on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
gaodayue commented on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575558056
FYI @vagetablechicken
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken removed a comment on issue
#2791: Refine .clang-format
Posted by GitBox <gi...@apache.org>.
vagetablechicken removed a comment on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575564300
In Google style, `ColumnLimit: 80`.
So each line of text in code should be at most 80 characters long. It will generate many reformatting diffs. Is it OK？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575591455
That's nice, I prefer sorting includes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] gaodayue edited a comment on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
gaodayue edited a comment on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575579937
> 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] gaodayue commented on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
gaodayue commented on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575579937
> 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #2791: Refine
.clang-format
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #2791: Refine .clang-format
URL: https://github.com/apache/incubator-doris/pull/2791#issuecomment-575564300
In Google style, `ColumnLimit: 80`.
So each line of text in your code should be at most 80 characters long. It will generate many reformatting diffs. Is it OK？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org