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

Making diktat consistent with own code style #318

Merged
merged 14 commits into from
Sep 25, 2020

Conversation

petertrr
Copy link
Member

@petertrr petertrr commented Sep 23, 2020

What's done:

  • Diktat-common now passes check
  • Workflow now runs diktat on all modules consequently

Related issue #33

Issues discovered:

#317 #320 #321 #324 #325

@codecov
Copy link

codecov bot commented Sep 24, 2020

Codecov Report

Merging #318 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #318   +/-   ##
=========================================
  Coverage     82.29%   82.30%           
  Complexity     1016     1016           
=========================================
  Files            52       52           
  Lines          2627     2628    +1     
  Branches        830      830           
=========================================
+ Hits           2162     2163    +1     
  Misses          169      169           
  Partials        296      296           
Flag Coverage Δ Complexity Δ
#unittests 82.30% <83.33%> (+<0.01%) 1016.00 <1.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...fn/diktat/common/config/rules/RulesConfigReader.kt 42.85% <ø> (ø) 4.00 <0.00> (ø)
...ktat/ruleset/rules/ClassLikeStructuresOrderRule.kt 80.59% <80.00%> (+0.29%) 26.00 <0.00> (ø)
...tlin/org/cqfn/diktat/ruleset/constants/Warnings.kt 96.70% <100.00%> (ø) 9.00 <1.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b733074...68a56bd. Read the comment docs.

### What's done:
* Code style
### What's done:
* Code style
### What's done:
* Update diktat_snapshot.yml
### What's done:
* Update diktat_snapshot.yml
### What's done:
* Update diktat_snapshot.yml
### What's done:
* Update diktat_snapshot.yml
### What's done:
* Update diktat_snapshot.yml
@petertrr petertrr marked this pull request as ready for review September 25, 2020 10:33
Copy link
Collaborator

@kentr0w kentr0w left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@orchestr7 orchestr7 left a comment

Choose a reason for hiding this comment

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

lgtm

(it.findChildByType(MODIFIER_LIST) == null || it.findLeafWithSpecificType(PRIVATE_KEYWORD) != null)
&& it.getIdentifierName()!!.text.contains(loggerPropertyRegex)
}.toMutableList()
val loggers = allProperties
Copy link
Member

Choose a reason for hiding this comment

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

I guess we will need to change this logic:
allProperties.filter
.foo()
.bar()

is valid

Copy link
Member Author

Choose a reason for hiding this comment

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

So like if first call in chain is multiline lamba, then it should start on the same line as initial receiver?

Copy link
Member

@orchestr7 orchestr7 Sep 25, 2020

Choose a reason for hiding this comment

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

that should be possible, not prohibited. Both cases should be valid:
a.foo
.foo

a
.foo
.foo

but a.foo.foo is invalid

Copy link
Member Author

Choose a reason for hiding this comment

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

Create #326 for this

Copy link
Collaborator

@aktsay6 aktsay6 left a comment

Choose a reason for hiding this comment

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

lgtm

@petertrr petertrr merged commit e8ca725 into master Sep 25, 2020
@petertrr petertrr deleted the feature/diktat-code-style branch September 25, 2020 13:43
run: |
mvn -B install -DskipTests=true
mvn -B antrun:run@diktat --pl diktat-common || echo "LintErrors encountered"
mvn -B antrun:run@diktat --pl diktat-test-framework || echo "LintErrors encountered"
Copy link
Member

@orchestr7 orchestr7 Sep 25, 2020

Choose a reason for hiding this comment

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

but why not to make echo "LintErrors encountered in test-framework"
"in diktat", e.t.c

Copy link
Member Author

Choose a reason for hiding this comment

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

I just thought it would be a small line in build log, used as a workaround to continue after error, and the important information is in maven output

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.

4 participants