Skip to content

Add comments with a list of regexes to ForbiddenComment#5981

Merged
cortinico merged 30 commits into
detekt:mainfrom
atulgpt:fixes-3508
May 21, 2023
Merged

Add comments with a list of regexes to ForbiddenComment#5981
cortinico merged 30 commits into
detekt:mainfrom
atulgpt:fixes-3508

Conversation

@atulgpt

@atulgpt atulgpt commented Apr 10, 2023

Copy link
Copy Markdown
Contributor

Fixes #3508

@codecov

codecov Bot commented Apr 10, 2023

Copy link
Copy Markdown

Codecov Report

Merging #5981 (6166297) into main (b925968) will increase coverage by 0.28%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #5981      +/-   ##
============================================
+ Coverage     84.66%   84.94%   +0.28%     
- Complexity     3836     4026     +190     
============================================
  Files           549      569      +20     
  Lines         13065    13498     +433     
  Branches       2305     2381      +76     
============================================
+ Hits          11061    11466     +405     
- Misses          868      869       +1     
- Partials       1136     1163      +27     
Impacted Files Coverage Δ
.../arturbosch/detekt/rules/style/ForbiddenComment.kt 98.63% <100.00%> (-1.37%) ⬇️

... and 61 files with indirect coverage changes

@cortinico cortinico changed the title Add valuePatterns list of regexes Add valuePatterns with a list of regexes to ForbiddenComment Apr 11, 2023
@cortinico cortinico added this to the 1.23.0 milestone Apr 11, 2023
@cortinico

Copy link
Copy Markdown
Member

CI is crashing on rule code here:

 Caused by: java.lang.StringIndexOutOfBoundsException: String index out of range: -2
	at java.lang.String.substring(String.java:1931)
	at io.gitlab.arturbosch.detekt.rules.style.ForbiddenComment.checkForbiddenComment(ForbiddenComment.kt:87)
	at io.gitlab.arturbosch.detekt.rules.style.ForbiddenComment.visitKtFile(ForbiddenComment.kt:65)

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>

@BraisGabin BraisGabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Just a comment about the naming. What do you think?

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the wait. Looks good on my side, I don't think there are blockers, just a few thoughts.

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
Add tc when comment is present in between
@BraisGabin BraisGabin changed the title Add valuePatterns with a list of regexes to ForbiddenComment Add comments with a list of regexes to ForbiddenComment May 12, 2023
@atulgpt

atulgpt commented May 19, 2023

Copy link
Copy Markdown
Contributor Author

Pushed 6 7 8 commits, mainly the docs, and some more test cases and minor fixes. Also noticed a few more things while writing the docs, please have a look.

Thanks a lot for contributing in this

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's aim to resolve the rest of the threads and GO!

@atulgpt

atulgpt commented May 19, 2023

Copy link
Copy Markdown
Contributor Author

Let's aim to resolve the rest of the threads and GO!

Yesss... It got stretched way too much but yes it was a nice experience to learn about new things. Thanks for your support

@TWiStErRob TWiStErRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Everything is resolved, @cortinico merge as you wish for 1.23

@cortinico
cortinico merged commit e65ef3b into detekt:main May 21, 2023
@atulgpt
atulgpt deleted the fixes-3508 branch May 22, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ForbiddenComment should allow patterns

6 participants