Skip to content

QL: add ql/repeated-word query#9825

Merged
erik-krogh merged 7 commits intogithub:mainfrom
erik-krogh:repeatedWord
Aug 10, 2022
Merged

QL: add ql/repeated-word query#9825
erik-krogh merged 7 commits intogithub:mainfrom
erik-krogh:repeatedWord

Conversation

@erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Jul 14, 2022 •
edited
Loading

Inspired by this comment. (Even though that's an .md file, which we won't detect).

I also fixed all the alerts (as per usual).

@erik-krogh erik-krogh added the no-change-note-required This PR does not need a change note label Jul 14, 2022
@erik-krogh erik-krogh marked this pull request as ready for review July 14, 2022 11:39
@erik-krogh erik-krogh requested review from a team as code owners July 14, 2022 11:39
tausbn
tausbn previously approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

QL and Python stuff LGTM. 👍

geoffw0
geoffw0 previously approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

I'm a bit surprised this doesn't require a whitelist of acceptable repeat words. But the results speak for themselves. 👍

@erik-krogh
Copy link
Contributor Author

erik-krogh commented Jul 14, 2022

I'm a bit surprised this doesn't require a whitelist of acceptable repeat words. But the results speak for themselves. 👍

There was a whole lot of unsigned long long and similar in an earlier version of the query.
But those went away by restricting the regular expression to only consider things that look like the middle of a sentence.

intrigus-lgtm
intrigus-lgtm reviewed Jul 14, 2022
View reviewed changes
@erik-krogh @intrigus-lgtm
fix typo
625e37a 
Co-authored-by: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com>
@erik-krogh erik-krogh dismissed stale reviews from geoffw0 and tausbn via 625e37a July 14, 2022 19:53
@erik-krogh erik-krogh mentioned this pull request Jul 14, 2022
Merged
@erik-krogh erik-krogh requested a review from tausbn July 14, 2022 21:39
michaelnebel
michaelnebel previously approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# LGTM
QL for QL looks plausible to me :-)

atorralba
atorralba previously approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Java 👍

@erik-krogh erik-krogh dismissed stale reviews from atorralba and michaelnebel via 559ec7b August 9, 2022 19:23
@erik-krogh
Copy link
Contributor Author

erik-krogh commented Aug 9, 2022

Merged with main to resolve a conflict.
I'll merge on the next approving review. (@github/codeql-ruby 👀 ?).

hmac
hmac approved these changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@hmac hmac left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ruby 👍 (and the other changes look good too, AFAICT)

@erik-krogh erik-krogh merged commit d008975 into github:main Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelnebel michaelnebel michaelnebel left review comments

@geoffw0 geoffw0 geoffw0 left review comments

@tausbn tausbn Awaiting requested review from tausbn

+3 more reviewers

@intrigus-lgtm intrigus-lgtm intrigus-lgtm left review comments

@hmac hmac hmac approved these changes

@atorralba atorralba atorralba left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C# C++ Go Java no-change-note-required This PR does not need a change note Python QL-for-QL Ruby

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@erik-krogh @hmac @tausbn @michaelnebel @atorralba @geoffw0 @intrigus-lgtm