-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add build task experimental rules #1307
Merged
romtsn
merged 22 commits into
pinterest:master
from
paul-dingemans:add-build-task-experimental-rules
Jan 21, 2022
Merged
Add build task experimental rules #1307
romtsn
merged 22 commits into
pinterest:master
from
paul-dingemans:add-build-task-experimental-rules
Jan 21, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ktlint should apply the dogfooding principal and only provide experimental rules that at least on the ktlint code base itself gives satisfiable results. Initially all experimental rules that cause violations have been disabled, so that a separate commit can be created to enable each specific rule.
For BaselineTests it was necessary to rename the files which are used for testing to have a non standard kotlin file extension. This prevents the files from being changed when running the ktlint formatting on the ktlint code base itself. Note that the baseline protection mechanism did work in this case and as of that has been removed from the command.
It is not yet possible to enable the rule as some violations are actually false positives. This will be solved by pinterest#1284
For now, it has been chosen to disallow trailing comma's instead of forcing them to be added. Reasons for this are two folded. The number of changes is considerably smaller. More importantly is that the benefit, with respect to avoiding future merge conflicts, seems not that big when scanning the code change which would result from forcing the trailing comma to be added.
romtsn
approved these changes
Dec 12, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 minor things, but LGTM otherwise, thanks.
There is no need for a separate build task to run the experimental rules. The experimental rules can be executed by default in the "ktlint" task. Also, the baseline has been fixed so there is no longer a need to use extension "_kt" for the baseline test files. Closes pinterest#1222
romtsn
reviewed
Dec 14, 2021
romtsn
reviewed
Dec 14, 2021
romtsn
reviewed
Dec 14, 2021
romtsn
reviewed
Dec 14, 2021
…perimental-rules # Conflicts: # CHANGELOG.md # build.gradle
Those source files all contain linting errors which have to be reported by unit tests. Therefore they should not be reported during a normal build because those should not be fixed as that would result in the tests to fail.
@romtsn Can you give it another try? I have fixed the tests. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Ktlint should apply the dogfooding principal and only provide experimental rules that at least on the ktlint code base itself gives satisfiable results.
In the intial commit of the PR all experimental rules that causes a violation have been disabled. In following commits, rules are enabled one by one to make the consequences of enabling a rule clear. See commit messages for further details about choices made. Most important choices:
positives. This will be solved by Remove lint warning about unexpected indentation outside of indent rule #1284 after which this rule can and should be enabled as well.
Checklist
CHANGELOG.md
is updated