It is done. In the same PR I wiped out checkstyle rules that became
redundant. There are still quite a few.
On Thu, Jun 28, 2018 at 10:02 AM Udi Meiri wrote:
> Python already has lint checks: :beam-sdks-python:lint
> There are autoformatting tools, which we don't use AFAIK:
>
Python already has lint checks: :beam-sdks-python:lint
There are autoformatting tools, which we don't use AFAIK:
https://github.com/myint/autoflake
https://github.com/hhatto/autopep8
On Thu, Jun 28, 2018 at 12:31 AM Daniel Kulp wrote:
>
>
> On Jun 28, 2018, at 6:01 AM, Kenneth Knowles wrote:
I don't think so. Checkstyle checks for a number of things beyond simple
formatting issues, (i.e. redundant imports).
On Thu, Jun 28, 2018 at 12:31 AM Daniel Kulp wrote:
>
>
> On Jun 28, 2018, at 6:01 AM, Kenneth Knowles wrote:
>
> So, there are a few modes of checkstyle failure that can be
> On Jun 28, 2018, at 6:01 AM, Kenneth Knowles wrote:
>
> So, there are a few modes of checkstyle failure that can be induced by this:
>
> - lines get too long because of wrap & indent logic
> - a lot of our HTML is actually already broken and doesn't have tags
> where it should; spotless
So, there are a few modes of checkstyle failure that can be induced by this:
- lines get too long because of wrap & indent logic
- a lot of our HTML is actually already broken and doesn't have tags
where it should; spotless adds a blank line which makes checkstyle notice
the errors
I think
+1!
Given this is the recommended default for Go projects (with it's own gofmt
tool) I'm for similar tooling for this in other languages.
I suspect we can add Gradle command to run gofmt over the go code too for a
consistent hook to run for beam code. It would save folks from needing to
set up
+1
I already used it in SQL module and it's great. It makes sense to apply on all
modules.
Thanks !
Regards
JB
Le 27 juin 2018 à 12:15, à 12:15, Kenneth Knowles a écrit:
>Hi all,
>
>I like readable code, but I don't like formatting it myself. And I
>_really_
>don't like discussing in code
OK, I opened https://github.com/apache/beam/pull/5797. This thread is still
less than a day, so there's no commitment yet. There's no big hurry - I can
always discard the autoformat and rerun it. I'm sure there will always be
conflicts so I will just have to rerun it and merge at some quiet period
+1 I'll throw in my support for auto-formatting, especially if the entire
project is auto-formatted in advance.
On Wed, Jun 27, 2018 at 10:53 AM Huygaa Batsaikhan
wrote:
> +1. Global auto-formatting is cool!
>
> On Wed, Jun 27, 2018 at 10:17 AM Kenneth Knowles wrote:
>
>> I just mean that
I just mean that because of how the tool works. But I guess if there were
discretion then two different people could end up with autoformatting that
disagrees, so again you get lines in the PR diff that aren't real changes.
Kenn
On Wed, Jun 27, 2018 at 10:16 AM Raghu Angadi wrote:
> On Wed,
On Wed, Jun 27, 2018 at 10:13 AM Kenneth Knowles wrote:
> Nope! No discretion allowed :-)
>
+1. Fair enough!
>
> On Wed, Jun 27, 2018 at 9:57 AM Raghu Angadi wrote:
>
>> +1.
>>
>> Wondering if it can be configured to reformat only what we care most
>> about (2 space indentation etc),
Nope! No discretion allowed :-)
On Wed, Jun 27, 2018 at 9:57 AM Raghu Angadi wrote:
> +1.
>
> Wondering if it can be configured to reformat only what we care most about
> (2 space indentation etc), allowing some discretion on the edges. An
> example of inconsistent formatting that ends up in my
+1.
Wondering if it can be configured to reformat only what we care most about
(2 space indentation etc), allowing some discretion on the edges. An
example of inconsistent formatting that ends up in my code:
---
anObject.someLongMethodName(arg_number_1,
It wasn't clear to me that the intent was to autoformat all the code from
the proposal initially. If thats the case, then the delta is quite small
typically.
Also, it would be easier if we recommended to users to run run
"./gradlew spotlessApply"
which will run spotless on all modules.
On Wed,
On Wed, Jun 27, 2018 at 9:31 AM Kenneth Knowles wrote:
> Luke: the proposal here solves exactly what you are talking about.
>
> The problem you describe happens when the PR author uses autoformat but
> the baseline is not already autoformatted. What I am proposing is to make
> sure the baseline
Luke: the proposal here solves exactly what you are talking about.
The problem you describe happens when the PR author uses autoformat but the
baseline is not already autoformatted. What I am proposing is to make sure
the baseline is already autoformatted, so PRs never have extraneous
formatting
I agree that PRs with formatting changes are impossible to review, but I
think that is exactly what this is trying to avoid. If this is run nightly,
some contributors will still autoformat their PRs and have a bunch of
unrelated changes in the diff. If the code is always well formatted (and
that
Luke: Anything that helps contributors and reviewers work better together -
+1! :D
On Wed, Jun 27, 2018 at 9:04 AM Lukasz Cwik wrote:
> If spotless is run against a PR that is already well formatted its a
> non-issue as the formatting changes are usually related to the change but I
> have
If spotless is run against a PR that is already well formatted its a
non-issue as the formatting changes are usually related to the change but I
have reviewed a few PRs that have 100s of lines of formatting change which
really obfuscates the work.
Instead of asking contributors to run spotless,
Good points, Dan. Checkstyle will still run, but just focused on the things
that go beyond format.
Kenn
On Wed, Jun 27, 2018 at 8:03 AM Etienne Chauchot
wrote:
> +1 !
> It's my custom to avoid reformatting to spare meaningless diff burden to
> the reviewer. Now it will be over, thanks.
>
>
+1 !It's my custom to avoid reformatting to spare meaningless diff burden to
the reviewer. Now it will be over, thanks.
Etienne
Le mardi 26 juin 2018 à 21:15 -0700, Kenneth Knowles a écrit :
> Hi all,
> I like readable code, but I don't like formatting it myself. And I _really_
> don't like
I’m +1 to the idea of using something to handle the auto-formatting of code.
HOWEVER, I want to make it clear that this will likely NOT remove checkstyle
from the builds as there are things that checkstyle checks that aren’t part of
this. Variable names, type names, javadoc things, etc….
++1
- This solves the big mismatch of autoformatting using eclipse's google
java style and the google-java-format plugin that makes a common source of
unneeded diffs part of the reviews.
- The spotless plugin makes this trivial to do and uniform.
We need to update the instructions on formatting
+1 to automating this with Gradle and never have problems with
formatting/forgetting about it again! :)
śr., 27 cze 2018 o 07:40 Tim Robertson
napisał(a):
> ++1
>
>
> On Wed, Jun 27, 2018 at 7:36 AM, Ahmet Altay wrote:
>
>> +1
>>
>> This is great idea. Does anyone know a similar tool for
++1
On Wed, Jun 27, 2018 at 7:36 AM, Ahmet Altay wrote:
> +1
>
> This is great idea. Does anyone know a similar tool for python? I believe
> go already has this as part of its tools with go fmt.
>
>
> On Tue, Jun 26, 2018 at 9:55 PM, Ankur Goenka wrote:
>
>> +1
>>
>> Intellij can help but
+1
This is great idea. Does anyone know a similar tool for python? I believe
go already has this as part of its tools with go fmt.
On Tue, Jun 26, 2018 at 9:55 PM, Ankur Goenka wrote:
> +1
>
> Intellij can help but still formatting is an additional thing to keep in
> mind. Enabling auto
+1
Intellij can help but still formatting is an additional thing to keep in
mind. Enabling auto formatting at gradle level will remove this additional
thing to keep in mind.
On Tue, Jun 26, 2018 at 9:49 PM Eugene Kirpichov
wrote:
> +1!
>
> In some cases the temptation to format code manually
+1!
In some cases the temptation to format code manually can be quite strong,
but the ease of just re-running the formatter after any change (especially
after global changes like class/method renames) overweighs it. I lost count
of the times when I wasted a precommit because some line became >100
+1! Remove guesswork :D
On Tue, Jun 26, 2018 at 9:15 PM Kenneth Knowles wrote:
> Hi all,
>
> I like readable code, but I don't like formatting it myself. And I
> _really_ don't like discussing in code review. "Spotless" [1] can enforce -
> and automatically apply - automatic formatting for
Hi all,
I like readable code, but I don't like formatting it myself. And I _really_
don't like discussing in code review. "Spotless" [1] can enforce - and
automatically apply - automatic formatting for Java, Groovy, and some
others.
This is not about style or wanting a particular layout. This is
30 matches
Mail list logo
