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

Wrapper in kts file #735

Merged
merged 12 commits into from
Feb 18, 2021
Merged

Wrapper in kts file #735

merged 12 commits into from
Feb 18, 2021

Conversation

kentr0w
Copy link
Collaborator

@kentr0w kentr0w commented Jan 28, 2021

What's done:

Created new rule, added tests

Actions checklist

### What's done:
   Created new rule, added tests
@kentr0w kentr0w added the enhancement New feature or request label Jan 28, 2021
@kentr0w kentr0w requested review from petertrr and aktsay6 January 28, 2021 11:41
### What's done:
   Fixed according our checkers
@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #735 (666c018) into master (4860902) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #735      +/-   ##
============================================
- Coverage     81.30%   81.28%   -0.02%     
- Complexity     2133     2148      +15     
============================================
  Files            96       97       +1     
  Lines          5510     5542      +32     
  Branches       1700     1706       +6     
============================================
+ Hits           4480     4505      +25     
- Misses          265      271       +6     
- Partials        765      766       +1     
Flag Coverage Δ Complexity Δ
unittests 81.28% <75.00%> (-0.02%) 0.00 <14.00> (ø)

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

Impacted Files Coverage Δ Complexity Δ
.../cqfn/diktat/ruleset/rules/chapter6/RunInScript.kt 72.41% <72.41%> (ø) 14.00 <14.00> (?)
...tlin/org/cqfn/diktat/ruleset/constants/Warnings.kt 97.87% <100.00%> (+0.01%) 12.00 <0.00> (ø)
...cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt 92.03% <100.00%> (+0.07%) 11.00 <0.00> (ø)
...otlin/org/cqfn/diktat/ruleset/utils/StringUtils.kt 88.57% <100.00%> (+0.33%) 0.00 <0.00> (ø)
.../diktat/ruleset/rules/chapter3/NullableTypeRule.kt 78.20% <0.00%> (+1.28%) 43.00% <0.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 4860902...666c018. Read the comment docs.

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.

Also need to add this rule to TOC

# Conflicts:
#	diktat-analysis.yml
#	diktat-rules/src/main/kotlin/generated/WarningNames.kt
#	diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
#	diktat-rules/src/main/resources/diktat-analysis-huawei.yml
#	diktat-rules/src/main/resources/diktat-analysis.yml
#	info/available-rules.md
### What's done:
   Fixed according our code style
### What's done:
   Fixed according our code style
### What's done:
   Fixed after review
### What's done:
   Fixed according to our code style
Copy link
Member

@petertrr petertrr left a comment

Choose a reason for hiding this comment

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

I think we should reconsider this rule text. We can add a condition for *.gradle.kts files and jsut *.kts files: gradle files should have more things allowed on top level. Please try your rule on build.gradle.kts files we have available (in diktat-gradle-plugin, in docs, probably from other our projects) to see if they yield sane results.

### What's done:
   Added new logic to gradle plugin
### What's done:
Fixed after review
### What's done:
   Fixed bug with file path
### What's done:
   Fixed according to our code style
Copy link
Member

@petertrr petertrr left a comment

Choose a reason for hiding this comment

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

LGTM, let's try

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

@kentr0w kentr0w merged commit 517e3d6 into master Feb 18, 2021
@kentr0w kentr0w deleted the feature/run-in-script branch February 18, 2021 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule regarding run in kotlin scripts
3 participants