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 comments with a list of regexes to ForbiddenComment #5981

Merged
merged 30 commits into from May 21, 2023

Conversation

atulgpt
Copy link
Contributor

@atulgpt atulgpt commented Apr 10, 2023

Fixes #3508

@codecov
Copy link

codecov bot commented Apr 10, 2023

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
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>
Copy link
Member

@BraisGabin BraisGabin left a comment

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?

Copy link
Member

@TWiStErRob TWiStErRob left a comment

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
Copy link
Contributor Author

atulgpt commented May 19, 2023

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

Copy link
Member

@TWiStErRob TWiStErRob left a comment

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
Copy link
Contributor Author

atulgpt commented May 19, 2023

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

Copy link
Member

@TWiStErRob TWiStErRob left a comment

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
23 checks passed
@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
5 participants