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 EditorConfig #3394

Closed
wants to merge 3 commits into from
Closed

Add EditorConfig #3394

wants to merge 3 commits into from

Conversation

wb9688
Copy link
Contributor

@wb9688 wb9688 commented Apr 8, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR adds an EditorConfig. EditorConfig is some standard to set certain code style related settings in different editors and IDEs. I've also changed all files (except gradlew.bat) to use LF line ending, replaced all tabs with 4 spaces, have a newline at the end, and to not have trailing whitespace. I didn't look at whether files that were formatted with spaces are actually formatted with 4 spaces, or whether the indentation is correct.

Agreement

.editorconfig Show resolved Hide resolved
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

I like those rules. I'm a little bit worried if changing all of the strings.xml files could cause problems with weblate. Also, is it ok to edit changelogs of older versions (I assume it is, I'm pointing out this just to make sure)? Other than that I think this can be merged.

@wb9688
Copy link
Contributor Author

wb9688 commented Apr 8, 2020

I'm a little bit worried if changing all of the strings.xml files could cause problems with weblate.

Maybe, cc @B0pol and @TobiGr. There are some issues that could actually be fixed through Weblate, however some others have to be fixed this way. I also noticed that some translations don't have %1$s and similar written correctly, however I didn't fix those. And I had to laugh when I saw the Flemish translation.

Also, is it ok to edit changelogs of older versions (I assume it is, I'm pointing out this just to make sure)?

Yes, F-Droid only looks at the changelog in the commit that was tagged for that release.

@wb9688 wb9688 added this to the 0.20.0 milestone Apr 15, 2020
wb9688 and others added 3 commits April 21, 2020 17:23
I didn't look at whether files are actually indented by 4 spaces though.
@wb9688 wb9688 requested a review from Stypox April 21, 2020 16:57
@wb9688 wb9688 modified the milestones: 0.20.0, 0.19.4 Apr 25, 2020
@wb9688 wb9688 mentioned this pull request May 1, 2020
5 tasks
@B0pol
Copy link
Member

B0pol commented May 14, 2020

I've a question: does this automatically set the rule for imports? Because often you remove the wildcard imports .*. But as it's automatically done by intellij, people revert that.
If it's possible to configure imports, please do it.

@wb9688
Copy link
Contributor Author

wb9688 commented May 14, 2020

@B0pol: No, that's not possible in EditorConfig, as that's some IntelliJ-specific setting, however we could add ij_java_class_count_to_use_import_on_demand = 999 and ij_java_names_count_to_use_import_on_demand = 999 and whatever the Kotlin setting is to it as IntelliJ supports their own non-standard settings in EditorConfig, which is technically invalid, but other EditorConfig implementations ignore it anyway, so we could safely add it. You could also manually set it in Settings -> Editor -> Code Style -> Java -> Imports, and we enforce not using wildcard imports through Checkstyle and Ktlint, and having alphabetically sorted imports in Kotlin through Ktlint (which IntelliJ doesn't do fully automatically, though that will be fixed by JetBrains/kotlin#3336 and for now we can just manually run the formatKtlint task when writing Kotlin).

@B0pol B0pol added the feature request Issue is related to a feature in the app label May 17, 2020
@Stypox Stypox modified the milestones: 0.19.4, 0.19.5 May 28, 2020
@wb9688 wb9688 removed this from the 0.19.6 milestone Jun 28, 2020
@wb9688 wb9688 closed this Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants