The Wayback Machine - https://web.archive.org/web/20220106232918/https://github.com/github/codeql/pull/7458
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

QL: add "modelling/modeling" to ql/non-us-spelling #7458

Merged
merged 5 commits into from Jan 4, 2022

Conversation

@erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Dec 20, 2021

/cc @intrigus-lgtm

I also made a patch thing that automatically fix the issues found by the new query.

@erik-krogh erik-krogh requested a review from tausbn as a code owner Dec 20, 2021
Copy link
Contributor

@hvitved hvitved left a comment

C# 👍

@intrigus-lgtm
Copy link
Contributor

@intrigus-lgtm intrigus-lgtm commented Dec 20, 2021

You could also add modelled/modeled.

tausbn
tausbn approved these changes Jan 3, 2022
Copy link
Contributor

@tausbn tausbn left a comment

Python looks good. One minor comment about the QL.

// analyses (as a noun) is fine
s.regexpMatch(".*analyse[^s].*") and
wrong = "analyse"
Copy link
Contributor

@tausbn tausbn Jan 3, 2022

Choose a reason for hiding this comment

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

Perhaps it would make more sense to use an explicit word-boundary match, so something like s.regexpMatch(".*\\b" + wrong + "\\b.*").

Copy link
Contributor Author

@erik-krogh erik-krogh Jan 4, 2022

Choose a reason for hiding this comment

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

Good point.
But I think I'll wait with that until we encounter FPs.

With the current approach we also catch when the bad spelling is used as a prefix/suffix.
(But luckily English is not like Danish, so that will be rare).

Copy link
Contributor

@MathiasVP MathiasVP left a comment

C++ 👍

@erik-krogh erik-krogh merged commit b996479 into github:main Jan 4, 2022
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.