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:
> https://github.com/
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 ind
> 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 tha
+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 the
+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 revi
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 becau
+1. Global auto-formatting is cool!
On Wed, Jun 27, 2018 at 10:17 AM Kenneth Knowles wrote:
> 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
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, Jun
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), allowing
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, Ju
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 i
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 c
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 is
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 revi
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, can
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.
>
> Etie
+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 disc
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…. A
++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 a
+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 pyth
++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 stil
+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 format
+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 ca
+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 Jav
30 matches
Mail list logo
