You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Joan Goyeau <jo...@goyeau.com> on 2018/05/09 22:31:02 UTC
Use of a formatter like Scalafmt
Hi,
Contributing to Kafka Streams' Scala API, I've been kinda lost on how
should I format my code.
I know formatting is the start of religion wars but personally I have no
preference at all. I just want consistency across the codebase, no
unnecessary formatting diffs in PRs and offload the formatting to a tool
that will do it for me and concentrate on what matters (not formatting).
So I opened the following PR where I put arbitrary rules in .scalafmt.conf
<https://github.com/apache/kafka/pull/4965/files#diff-8af3e1355c23c331ee2b848e12c5219f>
:
https://github.com/apache/kafka/pull/4965
Please let me know what do you think and if we can move this forward and
settle something.
Thanks
Re: Use of a formatter like Scalafmt
Posted by Joan Goyeau <jo...@goyeau.com>.
Thanks guys for your feedback.
Matthias, we can change the config to use JavaDoc (1 space) instead of the
Scala doc (2 space), that would limit the change indeed.
When I say we can apply this change module by module I mean we can specify
folders, so we could breakdown core too.
I updated the PR to include only the streams folder and the JavaDoc change,
see:
https://github.com/apache/kafka/pull/4965
That would be a good start, see how it goes and later move on to some other
modules.
Let me know your thoughts on the PR too about rewritings that you don't
like or like.
Thanks
On Thu, 10 May 2018, 05:18 Jeff Widman, <je...@jeffwidman.com> wrote:
> It certainly annoys me every time I open the code and my linter starts
> highlighting that some code is indented with spaces and some with tabs...
> increasing consistency across the codebase would be appreciated.
>
> On Wed, May 9, 2018 at 9:10 PM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Sounds good about doing this for Kafka streams scala first. Core is a bit
> > more complicated so may require more discussion.
> >
> > Ismael
> >
> > On Wed, 9 May 2018, 16:59 Matthias J. Sax, <ma...@confluent.io>
> wrote:
> >
> > > Joan,
> > >
> > > thanks for starting this initiative. I am overall +1
> > >
> > > However, I am worried about applying it to `core` module as the change
> > > is massive. For the Kafka Streams Scala module, that is new and was not
> > > released yet, I am +1.
> > >
> > > A personal thing about the style: the 2-space indention for JavaDocs is
> > > a little weird.
> > >
> > > /**
> > > *
> > > */
> > >
> > > is changed to
> > >
> > > /**
> > > *
> > > */
> > >
> > > Not sure if this can be fixed easily in the style file? If not, I am
> > > also fine with the change.
> > >
> > > This change also affect the license headers of many files and exposing
> > > that those use the wrong comment format anyway. They should use regular
> > > comments
> > >
> > > /*
> > > *
> > > */
> > >
> > > but not JavaDoc comments
> > >
> > > /**
> > > *
> > > */
> > >
> > > (We fixed this for Java source code in the past already -- maybe it's
> > > time to fix it for Scala code base, too.
> > >
> > >
> > >
> > > -Matthias
> > >
> > > On 5/9/18 4:45 PM, Joan Goyeau wrote:
> > > > Hi Ted,
> > > >
> > > > As far as I understand this is an issue for PRs and back porting the
> > > > changes to other branches.
> > > >
> > > > Applying the tool to the other branches should also resolve the
> > conflicts
> > > > as the formattings will match, leaving only the actual changes in the
> > > diffs.
> > > > That's what we did sometime ago at my work and it went quiet
> smoothly.
> > > >
> > > > If we don't want to do a big bang commit then I'm thinking we might
> > want
> > > to
> > > > make it gradually by applying it module by module?
> > > > This is one idea do you have any other?
> > > >
> > > > I know formatting sounds like the useless thing that doesn't matter
> > and I
> > > > totally agree with this, that's why I don't want to care about it
> while
> > > > coding.
> > > >
> > > > Thanks
> > > >
> > > > On Thu, 10 May 2018 at 00:15 Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > >> Applying the tool across code base would result in massive changes.
> > > >> How would this be handled ?
> > > >> -------- Original message --------From: Joan Goyeau <
> joan@goyeau.com>
> > > >> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org
> Subject:
> > > Use
> > > >> of a formatter like Scalafmt
> > > >> Hi,
> > > >>
> > > >> Contributing to Kafka Streams' Scala API, I've been kinda lost on
> how
> > > >> should I format my code.
> > > >> I know formatting is the start of religion wars but personally I
> have
> > no
> > > >> preference at all. I just want consistency across the codebase, no
> > > >> unnecessary formatting diffs in PRs and offload the formatting to a
> > tool
> > > >> that will do it for me and concentrate on what matters (not
> > formatting).
> > > >>
> > > >> So I opened the following PR where I put arbitrary rules in
> > > .scalafmt.conf
> > > >> <
> > > >>
> > > https://github.com/apache/kafka/pull/4965/files#diff-
> > 8af3e1355c23c331ee2b848e12c5219f
> > > >>>
> > > >> :
> > > >> https://github.com/apache/kafka/pull/4965
> > > >>
> > > >> Please let me know what do you think and if we can move this forward
> > and
> > > >> settle something.
> > > >>
> > > >> Thanks
> > > >>
> > > >
> > >
> > >
> >
>
>
>
> --
>
> *Jeff Widman*
> jeffwidman.com <http://www.jeffwidman.com/> | 740-WIDMAN-J (943-6265)
> <><
>
Re: Use of a formatter like Scalafmt
Posted by Jeff Widman <je...@jeffwidman.com>.
It certainly annoys me every time I open the code and my linter starts
highlighting that some code is indented with spaces and some with tabs...
increasing consistency across the codebase would be appreciated.
On Wed, May 9, 2018 at 9:10 PM, Ismael Juma <is...@juma.me.uk> wrote:
> Sounds good about doing this for Kafka streams scala first. Core is a bit
> more complicated so may require more discussion.
>
> Ismael
>
> On Wed, 9 May 2018, 16:59 Matthias J. Sax, <ma...@confluent.io> wrote:
>
> > Joan,
> >
> > thanks for starting this initiative. I am overall +1
> >
> > However, I am worried about applying it to `core` module as the change
> > is massive. For the Kafka Streams Scala module, that is new and was not
> > released yet, I am +1.
> >
> > A personal thing about the style: the 2-space indention for JavaDocs is
> > a little weird.
> >
> > /**
> > *
> > */
> >
> > is changed to
> >
> > /**
> > *
> > */
> >
> > Not sure if this can be fixed easily in the style file? If not, I am
> > also fine with the change.
> >
> > This change also affect the license headers of many files and exposing
> > that those use the wrong comment format anyway. They should use regular
> > comments
> >
> > /*
> > *
> > */
> >
> > but not JavaDoc comments
> >
> > /**
> > *
> > */
> >
> > (We fixed this for Java source code in the past already -- maybe it's
> > time to fix it for Scala code base, too.
> >
> >
> >
> > -Matthias
> >
> > On 5/9/18 4:45 PM, Joan Goyeau wrote:
> > > Hi Ted,
> > >
> > > As far as I understand this is an issue for PRs and back porting the
> > > changes to other branches.
> > >
> > > Applying the tool to the other branches should also resolve the
> conflicts
> > > as the formattings will match, leaving only the actual changes in the
> > diffs.
> > > That's what we did sometime ago at my work and it went quiet smoothly.
> > >
> > > If we don't want to do a big bang commit then I'm thinking we might
> want
> > to
> > > make it gradually by applying it module by module?
> > > This is one idea do you have any other?
> > >
> > > I know formatting sounds like the useless thing that doesn't matter
> and I
> > > totally agree with this, that's why I don't want to care about it while
> > > coding.
> > >
> > > Thanks
> > >
> > > On Thu, 10 May 2018 at 00:15 Ted Yu <yu...@gmail.com> wrote:
> > >
> > >> Applying the tool across code base would result in massive changes.
> > >> How would this be handled ?
> > >> -------- Original message --------From: Joan Goyeau <jo...@goyeau.com>
> > >> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org Subject:
> > Use
> > >> of a formatter like Scalafmt
> > >> Hi,
> > >>
> > >> Contributing to Kafka Streams' Scala API, I've been kinda lost on how
> > >> should I format my code.
> > >> I know formatting is the start of religion wars but personally I have
> no
> > >> preference at all. I just want consistency across the codebase, no
> > >> unnecessary formatting diffs in PRs and offload the formatting to a
> tool
> > >> that will do it for me and concentrate on what matters (not
> formatting).
> > >>
> > >> So I opened the following PR where I put arbitrary rules in
> > .scalafmt.conf
> > >> <
> > >>
> > https://github.com/apache/kafka/pull/4965/files#diff-
> 8af3e1355c23c331ee2b848e12c5219f
> > >>>
> > >> :
> > >> https://github.com/apache/kafka/pull/4965
> > >>
> > >> Please let me know what do you think and if we can move this forward
> and
> > >> settle something.
> > >>
> > >> Thanks
> > >>
> > >
> >
> >
>
--
*Jeff Widman*
jeffwidman.com <http://www.jeffwidman.com/> | 740-WIDMAN-J (943-6265)
<><
Re: Use of a formatter like Scalafmt
Posted by Ismael Juma <is...@juma.me.uk>.
Sounds good about doing this for Kafka streams scala first. Core is a bit
more complicated so may require more discussion.
Ismael
On Wed, 9 May 2018, 16:59 Matthias J. Sax, <ma...@confluent.io> wrote:
> Joan,
>
> thanks for starting this initiative. I am overall +1
>
> However, I am worried about applying it to `core` module as the change
> is massive. For the Kafka Streams Scala module, that is new and was not
> released yet, I am +1.
>
> A personal thing about the style: the 2-space indention for JavaDocs is
> a little weird.
>
> /**
> *
> */
>
> is changed to
>
> /**
> *
> */
>
> Not sure if this can be fixed easily in the style file? If not, I am
> also fine with the change.
>
> This change also affect the license headers of many files and exposing
> that those use the wrong comment format anyway. They should use regular
> comments
>
> /*
> *
> */
>
> but not JavaDoc comments
>
> /**
> *
> */
>
> (We fixed this for Java source code in the past already -- maybe it's
> time to fix it for Scala code base, too.
>
>
>
> -Matthias
>
> On 5/9/18 4:45 PM, Joan Goyeau wrote:
> > Hi Ted,
> >
> > As far as I understand this is an issue for PRs and back porting the
> > changes to other branches.
> >
> > Applying the tool to the other branches should also resolve the conflicts
> > as the formattings will match, leaving only the actual changes in the
> diffs.
> > That's what we did sometime ago at my work and it went quiet smoothly.
> >
> > If we don't want to do a big bang commit then I'm thinking we might want
> to
> > make it gradually by applying it module by module?
> > This is one idea do you have any other?
> >
> > I know formatting sounds like the useless thing that doesn't matter and I
> > totally agree with this, that's why I don't want to care about it while
> > coding.
> >
> > Thanks
> >
> > On Thu, 10 May 2018 at 00:15 Ted Yu <yu...@gmail.com> wrote:
> >
> >> Applying the tool across code base would result in massive changes.
> >> How would this be handled ?
> >> -------- Original message --------From: Joan Goyeau <jo...@goyeau.com>
> >> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org Subject:
> Use
> >> of a formatter like Scalafmt
> >> Hi,
> >>
> >> Contributing to Kafka Streams' Scala API, I've been kinda lost on how
> >> should I format my code.
> >> I know formatting is the start of religion wars but personally I have no
> >> preference at all. I just want consistency across the codebase, no
> >> unnecessary formatting diffs in PRs and offload the formatting to a tool
> >> that will do it for me and concentrate on what matters (not formatting).
> >>
> >> So I opened the following PR where I put arbitrary rules in
> .scalafmt.conf
> >> <
> >>
> https://github.com/apache/kafka/pull/4965/files#diff-8af3e1355c23c331ee2b848e12c5219f
> >>>
> >> :
> >> https://github.com/apache/kafka/pull/4965
> >>
> >> Please let me know what do you think and if we can move this forward and
> >> settle something.
> >>
> >> Thanks
> >>
> >
>
>
Re: Use of a formatter like Scalafmt
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Joan,
thanks for starting this initiative. I am overall +1
However, I am worried about applying it to `core` module as the change
is massive. For the Kafka Streams Scala module, that is new and was not
released yet, I am +1.
A personal thing about the style: the 2-space indention for JavaDocs is
a little weird.
/**
*
*/
is changed to
/**
*
*/
Not sure if this can be fixed easily in the style file? If not, I am
also fine with the change.
This change also affect the license headers of many files and exposing
that those use the wrong comment format anyway. They should use regular
comments
/*
*
*/
but not JavaDoc comments
/**
*
*/
(We fixed this for Java source code in the past already -- maybe it's
time to fix it for Scala code base, too.
-Matthias
On 5/9/18 4:45 PM, Joan Goyeau wrote:
> Hi Ted,
>
> As far as I understand this is an issue for PRs and back porting the
> changes to other branches.
>
> Applying the tool to the other branches should also resolve the conflicts
> as the formattings will match, leaving only the actual changes in the diffs.
> That's what we did sometime ago at my work and it went quiet smoothly.
>
> If we don't want to do a big bang commit then I'm thinking we might want to
> make it gradually by applying it module by module?
> This is one idea do you have any other?
>
> I know formatting sounds like the useless thing that doesn't matter and I
> totally agree with this, that's why I don't want to care about it while
> coding.
>
> Thanks
>
> On Thu, 10 May 2018 at 00:15 Ted Yu <yu...@gmail.com> wrote:
>
>> Applying the tool across code base would result in massive changes.
>> How would this be handled ?
>> -------- Original message --------From: Joan Goyeau <jo...@goyeau.com>
>> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org Subject: Use
>> of a formatter like Scalafmt
>> Hi,
>>
>> Contributing to Kafka Streams' Scala API, I've been kinda lost on how
>> should I format my code.
>> I know formatting is the start of religion wars but personally I have no
>> preference at all. I just want consistency across the codebase, no
>> unnecessary formatting diffs in PRs and offload the formatting to a tool
>> that will do it for me and concentrate on what matters (not formatting).
>>
>> So I opened the following PR where I put arbitrary rules in .scalafmt.conf
>> <
>> https://github.com/apache/kafka/pull/4965/files#diff-8af3e1355c23c331ee2b848e12c5219f
>>>
>> :
>> https://github.com/apache/kafka/pull/4965
>>
>> Please let me know what do you think and if we can move this forward and
>> settle something.
>>
>> Thanks
>>
>
Re: Use of a formatter like Scalafmt
Posted by Joan Goyeau <jo...@goyeau.com>.
Hi Ted,
As far as I understand this is an issue for PRs and back porting the
changes to other branches.
Applying the tool to the other branches should also resolve the conflicts
as the formattings will match, leaving only the actual changes in the diffs.
That's what we did sometime ago at my work and it went quiet smoothly.
If we don't want to do a big bang commit then I'm thinking we might want to
make it gradually by applying it module by module?
This is one idea do you have any other?
I know formatting sounds like the useless thing that doesn't matter and I
totally agree with this, that's why I don't want to care about it while
coding.
Thanks
On Thu, 10 May 2018 at 00:15 Ted Yu <yu...@gmail.com> wrote:
> Applying the tool across code base would result in massive changes.
> How would this be handled ?
> -------- Original message --------From: Joan Goyeau <jo...@goyeau.com>
> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org Subject: Use
> of a formatter like Scalafmt
> Hi,
>
> Contributing to Kafka Streams' Scala API, I've been kinda lost on how
> should I format my code.
> I know formatting is the start of religion wars but personally I have no
> preference at all. I just want consistency across the codebase, no
> unnecessary formatting diffs in PRs and offload the formatting to a tool
> that will do it for me and concentrate on what matters (not formatting).
>
> So I opened the following PR where I put arbitrary rules in .scalafmt.conf
> <
> https://github.com/apache/kafka/pull/4965/files#diff-8af3e1355c23c331ee2b848e12c5219f
> >
> :
> https://github.com/apache/kafka/pull/4965
>
> Please let me know what do you think and if we can move this forward and
> settle something.
>
> Thanks
>
Re: Use of a formatter like Scalafmt
Posted by Ted Yu <yu...@gmail.com>.
Applying the tool across code base would result in massive changes.
How would this be handled ?
-------- Original message --------From: Joan Goyeau <jo...@goyeau.com> Date: 5/9/18 3:31 PM (GMT-08:00) To: dev@kafka.apache.org Subject: Use of a formatter like Scalafmt
Hi,
Contributing to Kafka Streams' Scala API, I've been kinda lost on how
should I format my code.
I know formatting is the start of religion wars but personally I have no
preference at all. I just want consistency across the codebase, no
unnecessary formatting diffs in PRs and offload the formatting to a tool
that will do it for me and concentrate on what matters (not formatting).
So I opened the following PR where I put arbitrary rules in .scalafmt.conf
<https://github.com/apache/kafka/pull/4965/files#diff-8af3e1355c23c331ee2b848e12c5219f>
:
https://github.com/apache/kafka/pull/4965
Please let me know what do you think and if we can move this forward and
settle something.
Thanks