Skip to content

Conversation

@EduardoVaz06
Copy link
Contributor

This PR adds a setting on MixedNames rule to ignore certain names.

This PR implements a suggestion I have for MixedNames rule. In my company, we have certain patterns in names, so in some cases it's suggested by the rule to comply with the definition, and ends up not being compliant with our pattern. I think it would be good to have a list for names to be ignored.

For example, sometimes the rules suggest to be WParam, and sometimes WPARAM, due to the definition. If we have a pattern to always be WParam, this addition could help to ignore WParam name, and don't raise any issues.

It's up to you to know if this is a desired feature or not.

@cirras cirras requested a review from fourls November 7, 2024 02:48
Copy link
Collaborator

@fourls fourls left a comment

Choose a reason for hiding this comment

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

Hi @EduardoVaz06, thanks for making this contribution! I agree with your reasoning and think it's definitely appropriate to include in the core rule behaviour.

Implementation looks great, no problems I can see. I have a couple of suggestions for some extra tests.

@cirras
Copy link
Collaborator

cirras commented Dec 5, 2024

Hi @EduardoVaz06, this PR is a month old.
Do you need anything from us to address the current review feedback?

@EduardoVaz06
Copy link
Contributor Author

Hi @EduardoVaz06, this PR is a month old. Do you need anything from us to address the current review feedback?

Hi! Sorry for the huge delay. Soon i will make the changes requested and go on with de pr!

@cirras cirras force-pushed the AllowListToMixedNames branch from c99a81f to 0577853 Compare February 16, 2025 22:01
@cirras cirras requested a review from fourls February 16, 2025 22:01
@cirras cirras force-pushed the AllowListToMixedNames branch from 0577853 to f27798a Compare February 16, 2025 22:19
Copy link
Collaborator

@fourls fourls 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!

@cirras cirras merged commit 8fa8ce4 into integrated-application-development:master Feb 16, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants