Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kotlin: enforce style guide #27

Closed
rebello95 opened this issue Jun 3, 2019 · 5 comments
Closed

kotlin: enforce style guide #27

rebello95 opened this issue Jun 3, 2019 · 5 comments
Assignees
@buildbreaker
Labels
no stalebot platform/android Issues related to Android
Milestone
v0.1 "Antipasto"

Comments

@rebello95
Copy link
Contributor

Integrate with a linter on CI for Kotlin styling.

We all agree that our style guide should be encoded in linters, not up to reviewers eyes. CI should catch errors in a "check_format" run.
@rebello95 rebello95 added no stalebot platform/android Issues related to Android labels Jun 3, 2019
@mattklein123 mattklein123 added this to the v0.1 "Antipasto" milestone Jun 7, 2019
@rebello95 rebello95 self-assigned this Jun 11, 2019
@rebello95
Copy link
Contributor Author

We're currently considering using https://github.com/arturbosch/detekt

@buildbreaker
Copy link

There aren't any trivial CLI options with https://github.com/arturbosch/detekt. We can move forward quicker if we just use https://github.com/pinterest/ktlint

@buildbreaker
Copy link

Have some path forward with detekt actually. Prototyping this with bazel

@buildbreaker
Copy link

Going to use detekt without bazel since the integration is not trivial as I originally thought

@buildbreaker buildbreaker mentioned this issue Jun 15, 2019
Merged
buildbreaker pushed a commit that referenced this issue Jun 17, 2019
Add detekt for basic kotlin linting (#96)
2dbcd46 
#27

Adding a simple script and using openjdk:8-jdk Docker image. Open to moving some of the script dependencies to a Dockerfile and can create an issue for that if we are interested

Using https://github.com/arturbosch/detekt with the `--build-upon-default-config` flag which uses the [default config settings](https://github.com/arturbosch/detekt/blob/master/detekt-cli/src/main/resources/default-detekt-config.yml) and some customization on top (found in `.kotlinlint.yml`). This does not support `detekt-formatter` and that can be something we look into if we want to as well.

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Add detekt for kotlin linting
Risk Level: low
Testing: locally and CI
Docs Changes: Added information about what we use to lint and the file we use
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]
@rebello95
Copy link
Contributor Author

Done in #96

@Augustyniak Augustyniak mentioned this issue Nov 2, 2022
Closed
jpsim pushed a commit to envoyproxy/envoy that referenced this issue Nov 28, 2022
@jpsim
Add detekt for basic kotlin linting (#96)
398214b 
envoyproxy/envoy-mobile#27

Adding a simple script and using openjdk:8-jdk Docker image. Open to moving some of the script dependencies to a Dockerfile and can create an issue for that if we are interested

Using https://github.com/arturbosch/detekt with the `--build-upon-default-config` flag which uses the [default config settings](https://github.com/arturbosch/detekt/blob/master/detekt-cli/src/main/resources/default-detekt-config.yml) and some customization on top (found in `.kotlinlint.yml`). This does not support `detekt-formatter` and that can be something we look into if we want to as well.

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Add detekt for kotlin linting
Risk Level: low
Testing: locally and CI
Docs Changes: Added information about what we use to lint and the file we use
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit to envoyproxy/envoy that referenced this issue Nov 29, 2022
@jpsim
Add detekt for basic kotlin linting (#96)
f520b47 
envoyproxy/envoy-mobile#27

Adding a simple script and using openjdk:8-jdk Docker image. Open to moving some of the script dependencies to a Dockerfile and can create an issue for that if we are interested

Using https://github.com/arturbosch/detekt with the `--build-upon-default-config` flag which uses the [default config settings](https://github.com/arturbosch/detekt/blob/master/detekt-cli/src/main/resources/default-detekt-config.yml) and some customization on top (found in `.kotlinlint.yml`). This does not support `detekt-formatter` and that can be something we look into if we want to as well.

Signed-off-by: Alan Chiu <achiu@lyft.com>

For an explanation of how to fill out the fields, please see the relevant section
in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md)

Description: Add detekt for kotlin linting
Risk Level: low
Testing: locally and CI
Docs Changes: Added information about what we use to lint and the file we use
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@buildbreaker buildbreaker

Labels
no stalebot platform/android Issues related to Android
Projects
None yet
Milestone
v0.1 "Antipasto"
Development

No branches or pull requests
3 participants
@rebello95 @mattklein123 @buildbreaker