You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Martin Grund <ma...@databricks.com.INVALID> on 2022/10/14 07:36:03 UTC
Enforcing scalafmt on Spark Connect - connector/connect
Hi folks,
I'm reaching out to ask to gather input / consensus on the following
proposal: Since Spark Connect is effectively new code, I would like to
enforce scalafmt explicitly *only* on this module by adding a check in
`dev/lint-scala` that checks if there is a diff after running
./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -pl
connector/connect
I know that enforcing scalafmt is not desirable on the existing code base
but since the Spark Connect code is very new I'm thinking it might reduce
friction in the code reviews and create a consistent style.
In my previous code reviews where I have applied scalafmt I've
received feedback on the import grouping that scalafmt is changing
different from our default style. I've prepared a PR
https://github.com/apache/spark/pull/38252 to address this issue by
explicitly setting it in the scalafmt option.
Would you be supportive of enforcing scalafmt *only* on the Spark Connect
module?
Thanks
Martin
Re: Enforcing scalafmt on Spark Connect - connector/connect
Posted by Martin Grund <ma...@databricks.com.INVALID>.
I have prepared the following pull request that enforces scalafmt on the
Spark Connect module. Please feel free to have a look and leave comments.
If we're reaching consensus on the decision, I will take care of pushing a
PR as well for the previously mentioned website to add a small note on
using `dev/lint-scala` for local style checks.
Thanks
Martin
On Fri, Oct 14, 2022 at 11:09 AM Yikun Jiang <yi...@gmail.com> wrote:
> +1, I also think it's a good idea.
>
> BTW, we might also consider adding some notes about `lint-scala` in [1],
> just like `lint-python` in pyspark [2].
>
> [1] https://spark.apache.org/developer-tools.html
> [2]
> https://spark.apache.org/docs/latest/api/python/development/contributing.html
>
>
> Regards,
> Yikun
>
>
> On Fri, Oct 14, 2022 at 4:51 PM Hyukjin Kwon <gu...@gmail.com> wrote:
>
>> I personally like this idea. At least we now do this in PySpark, and it's
>> pretty nice that you can just forget about formatting it manually by
>> yourself.
>>
>> On Fri, 14 Oct 2022 at 16:37, Martin Grund
>> <ma...@databricks.com.invalid> wrote:
>>
>>> Hi folks,
>>>
>>> I'm reaching out to ask to gather input / consensus on the following
>>> proposal: Since Spark Connect is effectively new code, I would like to
>>> enforce scalafmt explicitly *only* on this module by adding a check in
>>> `dev/lint-scala` that checks if there is a diff after running
>>>
>>> ./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -pl
>>> connector/connect
>>>
>>> I know that enforcing scalafmt is not desirable on the existing code
>>> base but since the Spark Connect code is very new I'm thinking it might
>>> reduce friction in the code reviews and create a consistent style.
>>>
>>> In my previous code reviews where I have applied scalafmt I've
>>> received feedback on the import grouping that scalafmt is changing
>>> different from our default style. I've prepared a PR
>>> https://github.com/apache/spark/pull/38252 to address this issue by
>>> explicitly setting it in the scalafmt option.
>>>
>>> Would you be supportive of enforcing scalafmt *only* on the Spark
>>> Connect module?
>>>
>>> Thanks
>>> Martin
>>>
>>
Re: Enforcing scalafmt on Spark Connect - connector/connect
Posted by Yikun Jiang <yi...@gmail.com>.
+1, I also think it's a good idea.
BTW, we might also consider adding some notes about `lint-scala` in [1],
just like `lint-python` in pyspark [2].
[1] https://spark.apache.org/developer-tools.html
[2]
https://spark.apache.org/docs/latest/api/python/development/contributing.html
Regards,
Yikun
On Fri, Oct 14, 2022 at 4:51 PM Hyukjin Kwon <gu...@gmail.com> wrote:
> I personally like this idea. At least we now do this in PySpark, and it's
> pretty nice that you can just forget about formatting it manually by
> yourself.
>
> On Fri, 14 Oct 2022 at 16:37, Martin Grund
> <ma...@databricks.com.invalid> wrote:
>
>> Hi folks,
>>
>> I'm reaching out to ask to gather input / consensus on the following
>> proposal: Since Spark Connect is effectively new code, I would like to
>> enforce scalafmt explicitly *only* on this module by adding a check in
>> `dev/lint-scala` that checks if there is a diff after running
>>
>> ./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -pl
>> connector/connect
>>
>> I know that enforcing scalafmt is not desirable on the existing code base
>> but since the Spark Connect code is very new I'm thinking it might reduce
>> friction in the code reviews and create a consistent style.
>>
>> In my previous code reviews where I have applied scalafmt I've
>> received feedback on the import grouping that scalafmt is changing
>> different from our default style. I've prepared a PR
>> https://github.com/apache/spark/pull/38252 to address this issue by
>> explicitly setting it in the scalafmt option.
>>
>> Would you be supportive of enforcing scalafmt *only* on the Spark
>> Connect module?
>>
>> Thanks
>> Martin
>>
>
Re: Enforcing scalafmt on Spark Connect - connector/connect
Posted by Hyukjin Kwon <gu...@gmail.com>.
I personally like this idea. At least we now do this in PySpark, and it's
pretty nice that you can just forget about formatting it manually by
yourself.
On Fri, 14 Oct 2022 at 16:37, Martin Grund
<ma...@databricks.com.invalid> wrote:
> Hi folks,
>
> I'm reaching out to ask to gather input / consensus on the following
> proposal: Since Spark Connect is effectively new code, I would like to
> enforce scalafmt explicitly *only* on this module by adding a check in
> `dev/lint-scala` that checks if there is a diff after running
>
> ./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -pl
> connector/connect
>
> I know that enforcing scalafmt is not desirable on the existing code base
> but since the Spark Connect code is very new I'm thinking it might reduce
> friction in the code reviews and create a consistent style.
>
> In my previous code reviews where I have applied scalafmt I've
> received feedback on the import grouping that scalafmt is changing
> different from our default style. I've prepared a PR
> https://github.com/apache/spark/pull/38252 to address this issue by
> explicitly setting it in the scalafmt option.
>
> Would you be supportive of enforcing scalafmt *only* on the Spark Connect
> module?
>
> Thanks
> Martin
>