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.

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

Adds editorconfig for current code style #5727

Merged
merged 5 commits into from
Apr 15, 2022

Conversation

ericdecanini
Copy link
Contributor

@ericdecanini ericdecanini commented Apr 11, 2022

Draft for testing with the linter on the CI

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Adds an .editorconfig file reflecting the current code style

Motivation and context

Closes #5729

In order to make incremental changes to our current code style, it's best for us to have an initial editorconfig that reflects our current code style that we then make changes to later.

An issue encountered when doing this is there are areas of the project that don't follow even the current code style for reasons such as indentation, wrapping, etc.

Instead of changing these files which would result in a massive diff, I opted for disabling these rules temporarily from the linter for the duration we undergo this code style change.

The editorconfig file found here is one extracted from Android Studio preferences reflecting the current code style. It includes settings for other languages too other than Kotlin. At first I thought we didn't need this and removed them but this resulted in a bug where XML files were unintentionally formatted differently so I decided to keep them all

Screenshots / GIFs

N/A

Tests

The main test here in that the linter passes. See that we have no changes to our autoformatter for different file types (kotlin, xml)

I also tested the app doesn't crash on boot

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 12

Checklist

@github-actions
Copy link

github-actions bot commented Apr 11, 2022

Unit Test Results

114 files  ±0  114 suites  ±0   1m 23s ⏱️ +2s
201 tests ±0  201 ✔️ ±0  0 💤 ±0  0 ±0 
674 runs  ±0  674 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 9c696b8. ± Comparison against base commit f8a3f7f.

♻️ This comment has been updated with latest results.

@ericdecanini ericdecanini added the PR-Small PR with less than 20 updated lines label Apr 12, 2022
@ericdecanini ericdecanini marked this pull request as ready for review April 12, 2022 10:07
@ericdecanini ericdecanini changed the title Adds editorconfig for current code style [DRAFT] Adds editorconfig for current code style Apr 12, 2022
@ericdecanini ericdecanini requested review from a team and fedrunov and removed request for a team April 12, 2022 15:13
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ij_properties_key_value_delimiter = equals
ij_properties_spaces_around_key_value_delimiter = false

[.editorconfig]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inception!

# From https://github.com/pinterest/ktlint#custom-ktlint-specific-editorconfig-properties
# default IntelliJ IDEA style, same as alphabetical, but with "java", "javax", "kotlin" and alias imports in the end of the imports list
ij_kotlin_imports_layout=*,java.**,javax.**,kotlin.**,^
[*]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the command to generate this file OOI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL, thanks!

Copy link
Contributor

@fedrunov fedrunov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ericdecanini ericdecanini merged commit 3b454b9 into develop Apr 15, 2022
@ericdecanini ericdecanini deleted the task/eric/current-code-style-config branch April 15, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Small PR with less than 20 updated lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an editorconfig file for the current code style
3 participants