You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2019/12/24 14:28:36 UTC
[GitHub] [incubator-hudi] sev7e0 opened a new pull request #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
sev7e0 opened a new pull request #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130
## What is the purpose of the pull request
Refactor the codes based on scala codestyle BlockImportChecker rule
## Brief change log
- *Refactor the codes based on scala codestyle BlockImportChecker rule*
## Verify this pull request
This pull request is a trivial rework / code cleanup without any test coverage.
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] vinothchandar commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568837111
@leesf and @lamber-ken you are already considering this, but please no more new rules, that require manual clean up..
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] leesf commented on a change in pull request #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
leesf commented on a change in pull request #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#discussion_r361230620
##########
File path: style/scalastyle.xml
##########
@@ -119,7 +119,7 @@
<check level="error" class="org.scalastyle.scalariform.PublicMethodsHaveTypeChecker" enabled="false"/>
<check level="error" class="org.scalastyle.file.NewLineAtEofChecker" enabled="true"/>
<check level="error" class="org.scalastyle.file.NoNewLineAtEofChecker" enabled="false"/>
- <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
+ <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="true"/>
Review comment:
The IDEA will place multi import in block automatically, we need to fix it manually, is there any other IDEA settings to fix it automatically or disable block import? cc @lamber-ken
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] lamber-ken commented on a change in pull request
#1130: [HUDI-445]Refactor the codes based on scala codestyle
BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on a change in pull request #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#discussion_r361230697
##########
File path: style/scalastyle.xml
##########
@@ -119,7 +119,7 @@
<check level="error" class="org.scalastyle.scalariform.PublicMethodsHaveTypeChecker" enabled="false"/>
<check level="error" class="org.scalastyle.file.NewLineAtEofChecker" enabled="true"/>
<check level="error" class="org.scalastyle.file.NoNewLineAtEofChecker" enabled="false"/>
- <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
+ <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="true"/>
Review comment:
> The IDEA will place multi import in block automatically, we need to fix it manually, is there any other IDEA settings to fix it automatically or disable block import? cc @lamber-ken
Lets talk about it.
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] leesf commented on issue #1130: [HUDI-445]Refactor
the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
leesf commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568810464
@sev7e0 Hi, thanks for opening this PR and welcome aboard, right now the IDEA will merge relative imports in block automatically, e.g. `import org.apache.spark.sql.{DataFrame, SQLContext, SaveMode}` in `HoodieSparkSqlWriter.scala`, it is very convenient. However, if we enable the BlockImportChecker rule, we may need fix it manually, which would waste some time for developers, a related disscussion would be found https://lists.apache.org/thread.html/d966752dcabd3305a888785944cd1ac2985b17e48d10d1c1abbca1dd%40%3Cdev.hudi.apache.org%3E, So IMHO we would keep the rule as it is before finding a better solution, WDYT? would close the PR and you would work on other scala codestyle rule if you are insterested :) . CC @lamber-ken FYI.
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] lamber-ken commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568810707
> @sev7e0 Hi, thanks for opening this PR and welcome aboard, right now the IDEA will merge relative imports in block automatically, e.g. `import org.apache.spark.sql.{DataFrame, SQLContext, SaveMode}` in `HoodieSparkSqlWriter.scala`, it is very convenient. However, if we enable the BlockImportChecker rule, we may need fix it manually, which would waste some time for developers, a related disscussion would be found https://lists.apache.org/thread.html/d966752dcabd3305a888785944cd1ac2985b17e48d10d1c1abbca1dd%40%3Cdev.hudi.apache.org%3E, So IMHO we would keep the rule as it is before finding a better solution, WDYT? would close the PR and you would work on other scala codestyle rule if you are insterested :) . CC @lamber-ken FYI.
@sev7e You can choose other scala codestyle rule, this rule this is controversial.
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] sev7e0 edited a comment on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
sev7e0 edited a comment on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568819888
> > @sev7e0 Hi, thanks for opening this PR and welcome aboard, right now the IDEA will merge relative imports in block automatically, e.g. `import org.apache.spark.sql.{DataFrame, SQLContext, SaveMode}` in `HoodieSparkSqlWriter.scala`, it is very convenient. However, if we enable the BlockImportChecker rule, we may need fix it manually, which would waste some time for developers, a related disscussion would be found https://lists.apache.org/thread.html/d966752dcabd3305a888785944cd1ac2985b17e48d10d1c1abbca1dd%40%3Cdev.hudi.apache.org%3E, So IMHO we would keep the rule as it is before finding a better solution, WDYT? would close the PR and you would work on other scala codestyle rule if you are insterested :) . CC @lamber-ken FYI.
>
> @sev7e You can choose other scala codestyle rule, this rule this is controversial.
Yes, I also think this code style is not convenient for developers. I try to find other validation rules. If you have good suggestions, you can give me a hint.
@leesf @lamber-ken
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] leesf commented on issue #1130: [HUDI-445]Refactor
the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
leesf commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568894069
@sev7e0 close the PR as discussed above. :)
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] lamber-ken commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568761177
+1
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] lamber-ken commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
lamber-ken commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568827028
> Sorry, I didn't seriously consider this problem yesterday. I think we should turn off this check. It does bring about the problem of multi line introduction, which is inconsistent with the introduction mechanism of idea itself and brings more work to Hudi developers. I refer to several other Scala projects, e.g. `spark` `flink `. None of them use this check rule.
>
> now, I'm thinking of adding an explanation, like this.
>
> ```
> </check>
>
> <!-- ============================rules we don't want================================== -->
>
> <!-- It bring about the problem of multi line introduction -->
> <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
> </scalastyle>
> ```
You're welcome. 😄
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] lamber-ken edited a comment on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
lamber-ken edited a comment on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568827028
> Sorry, I didn't seriously consider this problem yesterday. I think we should turn off this check. It does bring about the problem of multi line introduction, which is inconsistent with the introduction mechanism of idea itself and brings more work to Hudi developers. I refer to several other Scala projects, e.g. `spark` `flink `. None of them use this check rule.
>
> now, I'm thinking of adding an explanation, like this.
>
> ```
> </check>
>
> <!-- ============================rules we don't want================================== -->
>
> <!-- It bring about the problem of multi line introduction -->
> <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
> </scalastyle>
> ```
You're welcome. 😄
Don't need to do that, just choose other rules to fix.
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] sev7e0 commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568819888
@
> > @sev7e0 Hi, thanks for opening this PR and welcome aboard, right now the IDEA will merge relative imports in block automatically, e.g. `import org.apache.spark.sql.{DataFrame, SQLContext, SaveMode}` in `HoodieSparkSqlWriter.scala`, it is very convenient. However, if we enable the BlockImportChecker rule, we may need fix it manually, which would waste some time for developers, a related disscussion would be found https://lists.apache.org/thread.html/d966752dcabd3305a888785944cd1ac2985b17e48d10d1c1abbca1dd%40%3Cdev.hudi.apache.org%3E, So IMHO we would keep the rule as it is before finding a better solution, WDYT? would close the PR and you would work on other scala codestyle rule if you are insterested :) . CC @lamber-ken FYI.
>
> @sev7e You can choose other scala codestyle rule, this rule this is controversial.
Yes, I also think this code style is not convenient for developers. I try to find other validation rules. If you have good suggestions, you can give me a hint.
@leesf @lamber-ken
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] sev7e0 commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568828140
> > Sorry, I didn't seriously consider this problem yesterday. I think we should turn off this check. It does bring about the problem of multi line introduction, which is inconsistent with the introduction mechanism of idea itself and brings more work to Hudi developers. I refer to several other Scala projects, e.g. `spark` `flink `. None of them use this check rule.
> > now, I'm thinking of adding an explanation, like this.
> > ```
> > </check>
> >
> > <!-- ============================rules we don't want================================== -->
> >
> > <!-- It bring about the problem of multi line introduction -->
> > <check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
> > </scalastyle>
> > ```
>
> You're welcome. 😄
> Don't need to do that, just choose other rules to fix.
Ok, Thanks! @lamber-ken @leesf
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] leesf closed pull request #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
leesf closed pull request #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130
----------------------------------------------------------------
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
[GitHub] [incubator-hudi] sev7e0 commented on issue #1130:
[HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
Posted by GitBox <gi...@apache.org>.
sev7e0 commented on issue #1130: [HUDI-445]Refactor the codes based on scala codestyle BlockImportChecker rule
URL: https://github.com/apache/incubator-hudi/pull/1130#issuecomment-568826317
Sorry, I didn't seriously consider this problem yesterday. I think we should turn off this check. It does bring about the problem of multi line introduction, which is inconsistent with the introduction mechanism of idea itself and brings more work to Hudi developers. I refer to several other Scala projects, e.g. `spark` `flink `. None of them use this check rule.
now, I'm thinking of adding an explanation, like this.
```xml
</check>
<!-- ============================rules we don't want================================== -->
<!-- It bring about the problem of multi line introduction -->
<check level="error" class="org.scalastyle.scalariform.BlockImportChecker" enabled="false"/>
</scalastyle>
```
----------------------------------------------------------------
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