You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@iceberg.apache.org by Anton Okolnychyi <ao...@apple.com.INVALID> on 2019/04/10 08:59:42 UTC
Re: Style guidelines proposal for Iceberg
Thanks for this effort, Matt!
I would be interested to help applying this throughout the code base. Let me know if there is anything I can do.
Cheers,
Anton
> On 22 Mar 2019, at 17:36, Matt Cheah <mc...@palantir.com> wrote:
>
> The checks would be run as part of the build, and would fail for any style violations.
>
> Discuss the conventions on the PR if you find particular changes that we should discuss. I included a summary of the common changes in the PR description – feel free to quote these and bring them up for discussion as well.
>
> -Matt Cheah
>
> From: Ryan Blue <rblue@netflix.com <ma...@netflix.com>>
> Reply-To: "rblue@netflix.com <ma...@netflix.com>" <rblue@netflix.com <ma...@netflix.com>>
> Date: Friday, March 22, 2019 at 9:27 AM
> To: Matt Cheah <mcheah@palantir.com <ma...@palantir.com>>
> Cc: "dev@iceberg.apache.org <ma...@iceberg.apache.org>" <dev@iceberg.apache.org <ma...@iceberg.apache.org>>
> Subject: Re: Style guidelines proposal for Iceberg
>
> Thanks for working on this, Matt! If I understand correctly, the checks are run as part of the build, so Travis CI builds will fail for style violations?
>
> Where should we discuss the specific conventions? Would you prefer in this thread or on the PR?
>
> On Thu, Mar 21, 2019 at 5:07 PM Matt Cheah <mcheah@palantir.com <ma...@palantir.com>> wrote:
>> Hi everyone,
>>
>> A prerequisite for us to release Iceberg is to have proper style guidelines that are enforced in continuous integration. I would like to propose adding code linting using an open-source linting toolkit called Baseline [github.com] <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dbaseline&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=DUGA0qbk25bjBQu5D7PGpIVkiK9BQyjoH5inFAkNu84&e=>.
>>
>> I have submitted a pull request [github.com] <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Diceberg_pull_143&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=IVj85rAKzD80_VyUhj5ggullU7aTqukD48adtl20qrM&e=> to integrate Baseline with the iceberg-api module. In that pull request, I describe some of the style conventions that were adopted that were not enforced before, as well as cases in which we deviate from Baseline’s style guidelines in favor of Iceberg’s prior opinions.
>>
>> Please look over the pull request and the proposed changes, and provide any feedback you may have on this mailing list thread or in pull request comments.
>>
>> Once we’re satisfied with the style rules we’re committed to holding to, I can submit patches to apply linting to all of the other modules, but would certainly appreciate help in working through these. Let me know if you would be interested in contributing to this effort as well.
>>
>> Thanks,
>>
>> -Matt Cheah
>
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
Re: Style guidelines proposal for Iceberg
Posted by RD <rd...@gmail.com>.
+1 . I can help out too.
-R
On Wed, Apr 10, 2019 at 9:35 AM Matt Cheah <mc...@palantir.com> wrote:
> I opened a list of Github Issues that all have *[Baseline]* in the issue
> description. We can use that to keep track of the modules we have yet to
> apply Baseline to, as well as have contributors take ownership of each
> module in parallel.
>
>
>
> To add Baseline linting, modify this line
> <https://github.com/apache/incubator-iceberg/blob/master/build.gradle#L113>
> to include the project that is being linted. Then run ./gradlew check to
> find all the linting errors as a result.
>
>
>
> Please comment on the relevant Github issue if you intend to work on it
> accordingly.
>
>
>
> Thanks!
>
>
>
> -Matt Cheah
>
>
>
> *From: *Ryan Blue <rb...@netflix.com>
> *Reply-To: *"rblue@netflix.com" <rb...@netflix.com>
> *Date: *Wednesday, April 10, 2019 at 9:30 AM
> *To: *Matt Cheah <mc...@palantir.com>
> *Cc: *Anton Okolnychyi <ao...@apple.com>, "dev@iceberg.apache.org" <
> dev@iceberg.apache.org>
> *Subject: *Re: Style guidelines proposal for Iceberg
>
>
>
> I merged #152. I think it would be great to get more reviews on #153 and
> others that have a lot of changes.
>
>
>
> On Wed, Apr 10, 2019 at 8:50 AM Matt Cheah <mc...@palantir.com> wrote:
>
> That would be great – right now we mostly need to merge https://github.com/apache/incubator-iceberg/pull/152
> [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Diceberg_pull_152&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=1xK9o4B0W9X4cicVRCBD40Re0D1sUAhBK-NsKdL-yn8&s=_nFc6Fd93dK8VBNmEaAdE9iJ5rZYTjH8OWaFDOH83i0&e=>
> so it would be great to get that reviewed. Once we’ve done that we can
> proceed to apply the style guidelines to the other projects. Here’s an
> example of applying them to iceberg-core: https://github.com/apache/incubator-iceberg/pull/153
> [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Diceberg_pull_153&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=1xK9o4B0W9X4cicVRCBD40Re0D1sUAhBK-NsKdL-yn8&s=o9ylcZz3Se2cEBwxPtJ8AHNpZ9-owpqj7Xn3PQD_KcM&e=>
>
>
>
> The first PR, #152, makes it easy to apply Baseline to a given project and
> to work on that project’s style corrections in isolation. Once that’s in,
> we could parallelize the efforts across the Iceberg modules. We could open
> Github issues for each of the modules that have to be updated, and then
> contributors can claim assignment of these issues.
>
>
>
> -Matt Cheah
>
>
>
> *From: *<ao...@apple.com> on behalf of Anton Okolnychyi <
> aokolnychyi@apple.com>
> *Date: *Wednesday, April 10, 2019 at 2:00 AM
> *To: *"dev@iceberg.apache.org" <de...@iceberg.apache.org>
> *Cc: *"rblue@netflix.com" <rb...@netflix.com>, Matt Cheah <
> mcheah@palantir.com>
> *Subject: *Re: Style guidelines proposal for Iceberg
>
>
>
> Thanks for this effort, Matt!
>
>
>
> I would be interested to help applying this throughout the code base. Let
> me know if there is anything I can do.
>
>
>
> Cheers,
>
> Anton
>
>
>
>
>
> On 22 Mar 2019, at 17:36, Matt Cheah <mc...@palantir.com> wrote:
>
>
>
> The checks would be run as part of the build, and would fail for any style
> violations.
>
>
>
> Discuss the conventions on the PR if you find particular changes that we
> should discuss. I included a summary of the common changes in the PR
> description – feel free to quote these and bring them up for discussion as
> well.
>
>
>
> -Matt Cheah
>
>
>
> *From: *Ryan Blue <rb...@netflix.com>
> *Reply-To: *"rblue@netflix.com" <rb...@netflix.com>
> *Date: *Friday, March 22, 2019 at 9:27 AM
> *To: *Matt Cheah <mc...@palantir.com>
> *Cc: *"dev@iceberg.apache.org" <de...@iceberg.apache.org>
> *Subject: *Re: Style guidelines proposal for Iceberg
>
>
>
> Thanks for working on this, Matt! If I understand correctly, the checks
> are run as part of the build, so Travis CI builds will fail for style
> violations?
>
>
>
> Where should we discuss the specific conventions? Would you prefer in this
> thread or on the PR?
>
>
>
> On Thu, Mar 21, 2019 at 5:07 PM Matt Cheah <mc...@palantir.com> wrote:
>
> Hi everyone,
>
>
>
> A prerequisite for us to release Iceberg is to have proper style
> guidelines that are enforced in continuous integration. I would like to
> propose adding code linting using an open-source linting toolkit called Baseline
> [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dbaseline&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=DUGA0qbk25bjBQu5D7PGpIVkiK9BQyjoH5inFAkNu84&e=>
> .
>
>
>
> I have submitted a pull request [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Diceberg_pull_143&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=IVj85rAKzD80_VyUhj5ggullU7aTqukD48adtl20qrM&e=>
> to integrate Baseline with the iceberg-api module. In that pull request,
> I describe some of the style conventions that were adopted that were not
> enforced before, as well as cases in which we deviate from Baseline’s style
> guidelines in favor of Iceberg’s prior opinions.
>
>
>
> Please look over the pull request and the proposed changes, and provide
> any feedback you may have on this mailing list thread or in pull request
> comments.
>
>
>
> Once we’re satisfied with the style rules we’re committed to holding to, I
> can submit patches to apply linting to all of the other modules, but would
> certainly appreciate help in working through these. Let me know if you
> would be interested in contributing to this effort as well.
>
>
>
> Thanks,
>
>
>
> -Matt Cheah
>
>
>
>
> --
>
> Ryan Blue
>
> Software Engineer
>
> Netflix
>
>
>
>
>
>
> --
>
> Ryan Blue
>
> Software Engineer
>
> Netflix
>
Re: Style guidelines proposal for Iceberg
Posted by Matt Cheah <mc...@palantir.com>.
I opened a list of Github Issues that all have [Baseline] in the issue description. We can use that to keep track of the modules we have yet to apply Baseline to, as well as have contributors take ownership of each module in parallel.
To add Baseline linting, modify this line to include the project that is being linted. Then run ./gradlew check to find all the linting errors as a result.
Please comment on the relevant Github issue if you intend to work on it accordingly.
Thanks!
-Matt Cheah
From: Ryan Blue <rb...@netflix.com>
Reply-To: "rblue@netflix.com" <rb...@netflix.com>
Date: Wednesday, April 10, 2019 at 9:30 AM
To: Matt Cheah <mc...@palantir.com>
Cc: Anton Okolnychyi <ao...@apple.com>, "dev@iceberg.apache.org" <de...@iceberg.apache.org>
Subject: Re: Style guidelines proposal for Iceberg
I merged #152. I think it would be great to get more reviews on #153 and others that have a lot of changes.
On Wed, Apr 10, 2019 at 8:50 AM Matt Cheah <mc...@palantir.com> wrote:
That would be great – right now we mostly need to merge https://github.com/apache/incubator-iceberg/pull/152 [github.com] so it would be great to get that reviewed. Once we’ve done that we can proceed to apply the style guidelines to the other projects. Here’s an example of applying them to iceberg-core: https://github.com/apache/incubator-iceberg/pull/153 [github.com]
The first PR, #152, makes it easy to apply Baseline to a given project and to work on that project’s style corrections in isolation. Once that’s in, we could parallelize the efforts across the Iceberg modules. We could open Github issues for each of the modules that have to be updated, and then contributors can claim assignment of these issues.
-Matt Cheah
From: <ao...@apple.com> on behalf of Anton Okolnychyi <ao...@apple.com>
Date: Wednesday, April 10, 2019 at 2:00 AM
To: "dev@iceberg.apache.org" <de...@iceberg.apache.org>
Cc: "rblue@netflix.com" <rb...@netflix.com>, Matt Cheah <mc...@palantir.com>
Subject: Re: Style guidelines proposal for Iceberg
Thanks for this effort, Matt!
I would be interested to help applying this throughout the code base. Let me know if there is anything I can do.
Cheers,
Anton
On 22 Mar 2019, at 17:36, Matt Cheah <mc...@palantir.com> wrote:
The checks would be run as part of the build, and would fail for any style violations.
Discuss the conventions on the PR if you find particular changes that we should discuss. I included a summary of the common changes in the PR description – feel free to quote these and bring them up for discussion as well.
-Matt Cheah
From: Ryan Blue <rb...@netflix.com>
Reply-To: "rblue@netflix.com" <rb...@netflix.com>
Date: Friday, March 22, 2019 at 9:27 AM
To: Matt Cheah <mc...@palantir.com>
Cc: "dev@iceberg.apache.org" <de...@iceberg.apache.org>
Subject: Re: Style guidelines proposal for Iceberg
Thanks for working on this, Matt! If I understand correctly, the checks are run as part of the build, so Travis CI builds will fail for style violations?
Where should we discuss the specific conventions? Would you prefer in this thread or on the PR?
On Thu, Mar 21, 2019 at 5:07 PM Matt Cheah <mc...@palantir.com> wrote:
Hi everyone,
A prerequisite for us to release Iceberg is to have proper style guidelines that are enforced in continuous integration. I would like to propose adding code linting using an open-source linting toolkit called Baseline [github.com].
I have submitted a pull request [github.com] to integrate Baseline with the iceberg-api module. In that pull request, I describe some of the style conventions that were adopted that were not enforced before, as well as cases in which we deviate from Baseline’s style guidelines in favor of Iceberg’s prior opinions.
Please look over the pull request and the proposed changes, and provide any feedback you may have on this mailing list thread or in pull request comments.
Once we’re satisfied with the style rules we’re committed to holding to, I can submit patches to apply linting to all of the other modules, but would certainly appreciate help in working through these. Let me know if you would be interested in contributing to this effort as well.
Thanks,
-Matt Cheah
--
Ryan Blue
Software Engineer
Netflix
--
Ryan Blue
Software Engineer
Netflix
Re: Style guidelines proposal for Iceberg
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
I merged #152. I think it would be great to get more reviews on #153 and
others that have a lot of changes.
On Wed, Apr 10, 2019 at 8:50 AM Matt Cheah <mc...@palantir.com> wrote:
> That would be great – right now we mostly need to merge
> https://github.com/apache/incubator-iceberg/pull/152 so it would be great
> to get that reviewed. Once we’ve done that we can proceed to apply the
> style guidelines to the other projects. Here’s an example of applying them
> to iceberg-core: https://github.com/apache/incubator-iceberg/pull/153
>
>
>
> The first PR, #152, makes it easy to apply Baseline to a given project and
> to work on that project’s style corrections in isolation. Once that’s in,
> we could parallelize the efforts across the Iceberg modules. We could open
> Github issues for each of the modules that have to be updated, and then
> contributors can claim assignment of these issues.
>
>
>
> -Matt Cheah
>
>
>
> *From: *<ao...@apple.com> on behalf of Anton Okolnychyi <
> aokolnychyi@apple.com>
> *Date: *Wednesday, April 10, 2019 at 2:00 AM
> *To: *"dev@iceberg.apache.org" <de...@iceberg.apache.org>
> *Cc: *"rblue@netflix.com" <rb...@netflix.com>, Matt Cheah <
> mcheah@palantir.com>
> *Subject: *Re: Style guidelines proposal for Iceberg
>
>
>
> Thanks for this effort, Matt!
>
>
>
> I would be interested to help applying this throughout the code base. Let
> me know if there is anything I can do.
>
>
>
> Cheers,
>
> Anton
>
>
>
>
>
> On 22 Mar 2019, at 17:36, Matt Cheah <mc...@palantir.com> wrote:
>
>
>
> The checks would be run as part of the build, and would fail for any style
> violations.
>
>
>
> Discuss the conventions on the PR if you find particular changes that we
> should discuss. I included a summary of the common changes in the PR
> description – feel free to quote these and bring them up for discussion as
> well.
>
>
>
> -Matt Cheah
>
>
>
> *From: *Ryan Blue <rb...@netflix.com>
> *Reply-To: *"rblue@netflix.com" <rb...@netflix.com>
> *Date: *Friday, March 22, 2019 at 9:27 AM
> *To: *Matt Cheah <mc...@palantir.com>
> *Cc: *"dev@iceberg.apache.org" <de...@iceberg.apache.org>
> *Subject: *Re: Style guidelines proposal for Iceberg
>
>
>
> Thanks for working on this, Matt! If I understand correctly, the checks
> are run as part of the build, so Travis CI builds will fail for style
> violations?
>
>
>
> Where should we discuss the specific conventions? Would you prefer in this
> thread or on the PR?
>
>
>
> On Thu, Mar 21, 2019 at 5:07 PM Matt Cheah <mc...@palantir.com> wrote:
>
> Hi everyone,
>
>
>
> A prerequisite for us to release Iceberg is to have proper style
> guidelines that are enforced in continuous integration. I would like to
> propose adding code linting using an open-source linting toolkit called Baseline
> [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dbaseline&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=DUGA0qbk25bjBQu5D7PGpIVkiK9BQyjoH5inFAkNu84&e=>
> .
>
>
>
> I have submitted a pull request [github.com]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Diceberg_pull_143&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=hzwIMNQ9E99EMYGuqHI0kXhVbvX3nU3OSDadUnJxjAs&m=K0Dm15Lx_yNduH68j7QgZiQZq5hqz44AhUi4ObrML80&s=IVj85rAKzD80_VyUhj5ggullU7aTqukD48adtl20qrM&e=>
> to integrate Baseline with the iceberg-api module. In that pull request,
> I describe some of the style conventions that were adopted that were not
> enforced before, as well as cases in which we deviate from Baseline’s style
> guidelines in favor of Iceberg’s prior opinions.
>
>
>
> Please look over the pull request and the proposed changes, and provide
> any feedback you may have on this mailing list thread or in pull request
> comments.
>
>
>
> Once we’re satisfied with the style rules we’re committed to holding to, I
> can submit patches to apply linting to all of the other modules, but would
> certainly appreciate help in working through these. Let me know if you
> would be interested in contributing to this effort as well.
>
>
>
> Thanks,
>
>
>
> -Matt Cheah
>
>
>
>
> --
>
> Ryan Blue
>
> Software Engineer
>
> Netflix
>
>
>
--
Ryan Blue
Software Engineer
Netflix
Re: Style guidelines proposal for Iceberg
Posted by Matt Cheah <mc...@palantir.com>.
That would be great – right now we mostly need to merge https://github.com/apache/incubator-iceberg/pull/152 so it would be great to get that reviewed. Once we’ve done that we can proceed to apply the style guidelines to the other projects. Here’s an example of applying them to iceberg-core: https://github.com/apache/incubator-iceberg/pull/153
The first PR, #152, makes it easy to apply Baseline to a given project and to work on that project’s style corrections in isolation. Once that’s in, we could parallelize the efforts across the Iceberg modules. We could open Github issues for each of the modules that have to be updated, and then contributors can claim assignment of these issues.
-Matt Cheah
From: <ao...@apple.com> on behalf of Anton Okolnychyi <ao...@apple.com>
Date: Wednesday, April 10, 2019 at 2:00 AM
To: "dev@iceberg.apache.org" <de...@iceberg.apache.org>
Cc: "rblue@netflix.com" <rb...@netflix.com>, Matt Cheah <mc...@palantir.com>
Subject: Re: Style guidelines proposal for Iceberg
Thanks for this effort, Matt!
I would be interested to help applying this throughout the code base. Let me know if there is anything I can do.
Cheers,
Anton
On 22 Mar 2019, at 17:36, Matt Cheah <mc...@palantir.com> wrote:
The checks would be run as part of the build, and would fail for any style violations.
Discuss the conventions on the PR if you find particular changes that we should discuss. I included a summary of the common changes in the PR description – feel free to quote these and bring them up for discussion as well.
-Matt Cheah
From: Ryan Blue <rb...@netflix.com>
Reply-To: "rblue@netflix.com" <rb...@netflix.com>
Date: Friday, March 22, 2019 at 9:27 AM
To: Matt Cheah <mc...@palantir.com>
Cc: "dev@iceberg.apache.org" <de...@iceberg.apache.org>
Subject: Re: Style guidelines proposal for Iceberg
Thanks for working on this, Matt! If I understand correctly, the checks are run as part of the build, so Travis CI builds will fail for style violations?
Where should we discuss the specific conventions? Would you prefer in this thread or on the PR?
On Thu, Mar 21, 2019 at 5:07 PM Matt Cheah <mc...@palantir.com> wrote:
Hi everyone,
A prerequisite for us to release Iceberg is to have proper style guidelines that are enforced in continuous integration. I would like to propose adding code linting using an open-source linting toolkit called Baseline [github.com].
I have submitted a pull request [github.com] to integrate Baseline with the iceberg-api module. In that pull request, I describe some of the style conventions that were adopted that were not enforced before, as well as cases in which we deviate from Baseline’s style guidelines in favor of Iceberg’s prior opinions.
Please look over the pull request and the proposed changes, and provide any feedback you may have on this mailing list thread or in pull request comments.
Once we’re satisfied with the style rules we’re committed to holding to, I can submit patches to apply linting to all of the other modules, but would certainly appreciate help in working through these. Let me know if you would be interested in contributing to this effort as well.
Thanks,
-Matt Cheah
--
Ryan Blue
Software Engineer
Netflix