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

refactor: convert a public string to a property #308

Merged
merged 3 commits into from
Oct 6, 2024
Merged

Conversation

shpaass
Copy link
Owner

@shpaass shpaass commented Oct 6, 2024

Fixed the suppressed CA2211.

Also noticed that I wrote camelCase properties as the rule when they actually are not PascalCase, only because the legacy code uses them. I fixed that by removing this part from the code style.
Essentially, I suggest to use PascalCase properties if the class does not use camelCase ones.

I've also fixed the violations of the inspection IDE1006.

It came to my attention that we have violations of the inspection
that is responsible for custom rules: IDE1006.

After browsing editorconfig, I discovered that we have a couple
custom rules. One of them was explained as "Use camel case
for local variable".

This commit makes sure that any local functions and constants are
camelCase.

To reiterate, that does not change the code style. It applies
already-existing rules.
Use PascalCase properties if there are no camelCase ones around.

There is no reason for us to be unconventional by definition
regarding properties.

If the class uses camelCase properties, then we use camelCase.
If the class does not have camelCase properties, then we use
PascalCase properties.
That is done to get rid of the warning CA2211 that was previously
suppressed for the line where the string was defined.
@shpaass
Copy link
Owner Author

shpaass commented Oct 6, 2024

QA: checked the save, change, and load of a pY project.

@shpaass shpaass merged commit c333269 into master Oct 6, 2024
1 check passed
@shpaass shpaass deleted the string-to-property branch October 6, 2024 17:11
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.

1 participant