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

Add yarn lock file and Gradle verification tasks #35

Merged
merged 2 commits into from Dec 1, 2021

Conversation

OliverO2
Copy link
Contributor

As I started to work on this (more to come), I wanted to stabilize builds and avoid unpleasant surprises by npm package "updates". Please check if that's OK.

Copy link
Owner

@mpetuska mpetuska left a comment

Choose a reason for hiding this comment

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

Looks OK, but can you convert it to precompiled scrip plugin in here (see some examples from there already if you've never done that)

@OliverO2
Copy link
Contributor Author

OliverO2 commented Dec 1, 2021

OK, cool, I'll look into that.

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

BTW I sometimes miss GH notifications. Feel free to ping me on slack if you want a quick review ;)

@OliverO2
Copy link
Contributor Author

OliverO2 commented Dec 1, 2021

Seems to work as expected, should be ready to merge.

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

Can you fix code scanning workings regarding line lengths and wildcard imports? I've dismissed the others. Otherwise looks good.

@OliverO2
Copy link
Contributor Author

OliverO2 commented Dec 1, 2021

Where can I find out what the expectations are with respect to line length and star imports?

Actually, I'm struggling with IntelliJ's handling of that .editorconfig:

  • IntelliJ does not seem to honor ij_kotlin_name_count_to_use_star_import.
  • It also tells me about continuation_indent_size: "The property is not supported".
  • It fails when reversing a 2-space indentation (I always have to delete space characters one by one, as the automation just produces garbage).

I favor consistency, but I also like to configure as little as possible and neither fight the machine nor the larger community. With Python, for example, I have been using Black, which is absolutely strict. With Kotlin, I use IntelliJ's default code style, which allows more leeway where to put line breaks, but otherwise is pretty consistent. Actually, I find the current two-space indentation rather hard to read.

Regarding star imports, I happen to run into cases where the right extension function import is missing and IntelliJ is unwilling to suggest a proper fix. When discussing this with some JetBrains teams (serialization or Ktor, I don't remember), the answer was something along, "you are supposed to use star imports all the time".

Do you see a possibility to go more with the "standard" (4 spaces indentation, default import settings) and reduce the amount of manual intervention?

If not, could you provide a style guide detailing deviations from the Kotlin Coding conventions, so that I can tell in advance what's expected?

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

Yeah, ij support of .editorconfig is very poor. Here's my usual flow (assuming default ij setup):

  1. Adjust kotlin style in ij and disable wildstar imports and remove all package ordering rules
  2. Configure ij commit dialog to only run import optimisation and cleanup (no reformatting)
  3. Run ./gradlew spotlessApply before commit.
  4. Profit??

Aditionally you can install ktfmt ij plugin to handle formatting on save.

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

I'll make a note to provide more info on contributing in the readme.

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

As for inspections, I'm guessing you don't see detekt comments in PR... I'll fix them myself until I figure out how to make them public.

@mpetuska mpetuska merged commit c29b4e5 into mpetuska:master Dec 1, 2021
@OliverO2
Copy link
Contributor Author

OliverO2 commented Dec 1, 2021

Well, I do see some comments, but only sloppily worded ones which fail to provide essential information:

Line detected that is longer than the defined maximum line length in the code style.

@mpetuska
Copy link
Owner

mpetuska commented Dec 1, 2021

Oh, good to know! In aany case ./gradlew spotlessApply should fix everything save for imports.

@OliverO2 OliverO2 deleted the feature/yarn-lock branch December 2, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants