Skip to content

addons/namingng.py: Backward compatibility with addons/naming.py#5846

Closed
mvds00 wants to merge 1 commit into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq12
Closed

addons/namingng.py: Backward compatibility with addons/naming.py#5846
mvds00 wants to merge 1 commit into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq12

Conversation

@mvds00
Copy link
Copy Markdown
Contributor

@mvds00 mvds00 commented Jan 5, 2024

In case naming.py cannot be simply removed, due to it being in active use, this PR suggests a path forward, merging namingng.py and naming.py, with little risk.

A next step could be to drop namingng.py and s/namingng/naming/g throughout the project.

@mvds00 mvds00 mentioned this pull request Jan 5, 2024
Merged
@mvds00 mvds00 force-pushed the pullreq12 branch 2 times, most recently from a64fdc2 to 87bfe78 Compare January 5, 2024 23:50
@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 6, 2024

Note that the failing check is due to the issue fixed by #5847

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jan 6, 2024

Note that the failing check is due to the issue fixed by #5847

these errors does not look related to #5847

https://github.com/danmar/cppcheck/actions/runs/7427881828/job/20227959060?pr=5846

@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 6, 2024 •
edited
Loading

Note that the failing check is due to the issue fixed by #5847

these errors does not look related to #5847

https://github.com/danmar/cppcheck/actions/runs/7427881828/job/20227959060?pr=5846

You are right. The suppressions don't work anymore because the error id [naming-varname] in the output is now [naming-namingConvention]. Brutal. What do you prefer: should I update .selfcheck_suppressions to suppress the new error id, or should I add another compatibility feature to namingng.py to make it more closely act like naming.py, even matching the error ids?

Or maybe you could first answer the bigger question: what do you think about this naming/namingng development, and which direction would you like it to go? What behaviors/interfaces are important to keep and what can we safely change?

@mvds00
addons/namingng.py: Implement backward compatibility with addons/nami…
1f67073 
…ng.py.

Merging naming.py into namingng.py reduces redundant code and leverages
argument validation and the unit tests for namingng.py, with the aim to improve
maintainability.

naming.py supported RE_CONST to check constants, which is now a feature of
namingng.py as well; the unit test is expanded to cover the feature.

naming.py has been updated to call through to namingng.py.

Suppressions are updated to suppres the new namingng error id
[naming-namingConvention].

The unit tests for naming.py are kept as-is.
@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 7, 2024

Or maybe you could first answer the bigger question: what do you think about this naming/namingng development, and which direction would you like it to go? What behaviors/interfaces are important to keep and what can we safely change?

Not all issues were solved with suppressions. I've been trying to make namingng.py behave like naming.py, but their approaches are hard to unify. naming.py considers const-ness in a way that namingng.py cannot do now. Going forward I'm thinking of adding a new set of configuration options to namingng.py, that would allow it to be configured in a naming.py-compatible way.

@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 8, 2024

This PR will become part of #5852

@mvds00 mvds00 closed this Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mvds00 @danmar