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 2022/04/18 03:12:18 UTC

[GitHub] [incubator-doris] leo65535 opened a new pull request, #9072: [Improvement] Remove redundant imports

leo65535 opened a new pull request, #9072:
URL: https://github.com/apache/incubator-doris/pull/9072

   # Proposed changes
   
   Add `RedundantImport` `EmptyStatement` style rule.
   
   
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jackwener commented on a diff in pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#discussion_r857120717


##########
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/BulkLoadJob.java:
##########
@@ -55,10 +55,6 @@
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
-import org.apache.commons.lang.StringUtils;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-

Review Comment:
   They shouldn't be removed.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] leo65535 commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101186975

   > I think we should do `format` job together in community after community reach consensus.
   > 
   > related issue #8985
   
   Hi @jackwener, we can't do `format` job in just only one issuse, because it's will be a big job, we can split it to several sub tasks, like add `RedundantImport` `EmptyStatement` style rule one by one.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1112847495

   PR approved by at least one committer and no changes requested.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jackwener commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1112038476

   @leo65535 Yes, Be format is in the schedule. Now it's wrong.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1110497354

   Hi @leo65535 , please rebase the master to pass the check.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] leo65535 commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101192436

   > The doris dev email also is discussing this thing.
   
   emm, forgot to subscribe the dev email, I will subscribe the dev email soon.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yangzhg commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
yangzhg commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101109146

   Pay attention to the UT failed


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jackwener commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101156801

   I think we should do `format` job together in community after community reach consensus.
   
   related issue #8985 
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] leo65535 commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101186032

   Thanks @yangzhg for your advice , but I can build doris locally.
   ```
   [INFO] ------------------------------------------------------------------------
   [INFO] Reactor Summary for fe-common 1.0-SNAPSHOT:
   [INFO] 
   [INFO] fe-common .......................................... SUCCESS [ 26.373 s]
   [INFO] spark-dpp .......................................... SUCCESS [ 14.103 s]
   [INFO] fe-core ............................................ SUCCESS [01:06 min]
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  01:47 min
   [INFO] Finished at: 2022-04-18T07:33:42Z
   [INFO] ------------------------------------------------------------------------
   ***************************************
   Successfully build Doris
   ***************************************
   ```


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jackwener commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1101190058

   > Hi @jackwener, we can't do format job in just only one issuse, because it's will be a big job, we can split it to several sub tasks, like add RedundantImport EmptyStatement style rule one by one.
   
   Yes, I agree this point. But we're better off dealing with this intensively over a period of time.
   
   Because PR about `format` will influence the git history. We should make `format period` short as possible. 
   
   So, I think consensus is important. 
   
   BTW, Thanks for your contributionā¤šŸ˜ƒ, The doris dev email also is discussing this thing.
   
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jackwener commented on a diff in pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#discussion_r857122719


##########
fe/fe-core/src/main/java/org/apache/doris/load/loadv2/BulkLoadJob.java:
##########
@@ -55,10 +55,6 @@
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
-import org.apache.commons.lang.StringUtils;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-

Review Comment:
   It's my mistake, they are redundant.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1107836910

   PR approved by anyone and no changes requested.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] leo65535 commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1112026165

   Thanks @jackwener @morningman, I had rebased the master branch, but it seems the master branch has some cpp format issue.
   
   https://github.com/apache/incubator-doris/runs/6208605558?check_suite_focus=true


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman closed pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
morningman closed pull request #9072: [Improvement] Remove redundant imports
URL: https://github.com/apache/incubator-doris/pull/9072


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1114185769

   Hi @leo65535 , sorry that I can't merge this PR. We are plan to refactor the codestyle of FE step by step.
   The discussion thread is here: https://lists.apache.org/thread/yxjplc7v6sb0kzc9jxr1kl1xy2v8ctr3
   
   I will close this PR, and the issue will be handled, started from this: #9328 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] leo65535 commented on pull request #9072: [Improvement] Remove redundant imports

Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #9072:
URL: https://github.com/apache/incubator-doris/pull/9072#issuecomment-1114216233

   Got, thanks @morningman 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org