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

addons/namingng.py: Add tests for include guards, config file validation. #5815

Merged
merged 2 commits into from
Jan 2, 2024

Conversation

mvds00
Copy link
Contributor

@mvds00 mvds00 commented Dec 31, 2023

Include guard naming can be validated against various patterns:

  • prefixes/suffixes (_FILE_H, PROJECT_FILE_H, FILE_H_)
  • basename/full path (FILE_H, SUB_DIR_INC_FILE_H)
  • upper- or lowercase (FILE_H, file_h)
  • any combination of the above (project_sub_dir_inc_file_h_)

A regexp can be specified to match header filenames. The example matches any filename not starting with / and ending with .h, intended to match C header files while exluding system files.

The test is not limited to naming only; validity and presence of include guards can also be tested by setting "required":true in the config file.

Enabling this feature requires adding the key "include_guard" to the namingng config file used.

The namingng unit test is extended to test various features of the include guard test.

Also, config handling is improved, adding (superficial) validation and a unit test.

Include guard naming can be validated against various patterns:
- prefixes/suffixes (_FILE_H, PROJECT_FILE_H, FILE_H_)
- basename/full path (FILE_H, SUB_DIR_INC_FILE_H)
- upper- or lowercase (FILE_H, file_h) or case kept as-is (File_h)
- any combination of the above (project_sub_dir_inc_file_h_)

A regexp can be specified to match header filenames. The default matches any
filename not starting with / and ending with .h, intended to match C header
files while exluding system files.

The check is not limited to naming only; validity and presence of include
guards can also be tested by setting "required":true in the config file.

Enabling this feature requires adding the key "include_guard" to the namingng
config file.

The namingng unit test is extended to test various features of the include
guard check.
The config file is parsed and (superficially) validated, before starting
validation. Various errors are checked and reported, along with a non-zero exit
status.

After parsing and type validation, the config values are stored in an object,
so they can be referenced as object members instead of dict keys
(`conf.variable` instead of `conf["RE_VARNAME"]`). This separates config syntax
from application code.

A unit test is added to test config file validation.

The example config JSON in namingng.py is fixed.
@mvds00 mvds00 changed the title addons/namingng.py: Add tests for include guards. addons/namingng.py: Add tests for include guards, config file validation. Jan 1, 2024
Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

lgtm.. if you feel satisfied we can merge this..

@mvds00
Copy link
Contributor Author

mvds00 commented Jan 2, 2024

lgtm.. if you feel satisfied we can merge this..

Great. I cannot think of anything else to add or improve - it was used to clean up a medium-sized project already.

@chrchr-github chrchr-github merged commit 98b9244 into danmar:main Jan 2, 2024
68 checks passed
@firewave
Copy link
Collaborator

firewave commented Jan 5, 2024

Any behavior (breaking) changes, new features and fixes should be documented in releasenotes.txt. In case of (fixed) bugs you should also file a Trac ticket so they are being tracked. The same would also be good for (missing) features.

Also the addon should be documented in addons/README.md (or via doxygen or the doc folder - I have not checked how that is actually being used).

We (including external contributors) have been rather lax at both in the past and we (at least me) try to be more strict on this nowadays.

@mvds00
Copy link
Contributor Author

mvds00 commented Jan 5, 2024

Any behavior (breaking) changes, new features and fixes should be documented in releasenotes.txt. In case of (fixed) bugs you should also file a Trac ticket so they are being tracked. The same would also be good for (missing) features.

Also the addon should be documented in addons/README.md (or via doxygen or the doc folder - I have not checked how that is actually being used).

We (including external contributors) have been rather lax at both in the past and we (at least me) try to be more strict on this nowadays.

Sounds good. I will prepare a pull request, addressing:

  • addons/README.md documenting namingng.py
  • releasenotes.txt describing recent changes to namingng.py
  • additional unit test for namingng config without function/variable prefixes
  • reinstating unit test for standalone mode (maybe as part of existing unit test)

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