Skip to content

Conversation

@samopolacek
Copy link
Contributor

Cppcheck does not report that cppcheck build dir does not exist and also does not report any write issues to the non-existent directory.

This means that cppcheck build dir is actually not used.

We should either create the directory or fail.

@chrchr-github
Copy link
Collaborator

On which platform does this happen? It works fine on Windows.

@samopolacek
Copy link
Contributor Author

I am running cppcheck on Debian 12.

@chrchr-github
Copy link
Collaborator

Upon further review, the directory has to be created beforehand on Windows as well.

@firewave
Copy link
Collaborator

Please add a cli Python unit test for this.

Also this should be implemented via a function in the Path helper class.

@samopolacek samopolacek marked this pull request as draft July 28, 2023 09:30
@samopolacek
Copy link
Contributor Author

samopolacek commented Jul 28, 2023

  • Add a function to Path helper class
  • Add cli UT

@samopolacek samopolacek force-pushed the build_dir_existence branch from 6765276 to 50d1c87 Compare July 28, 2023 09:47
@samopolacek samopolacek force-pushed the build_dir_existence branch from efa8e0f to 492791c Compare July 28, 2023 11:41
@samopolacek samopolacek marked this pull request as ready for review July 28, 2023 12:33
@samopolacek
Copy link
Contributor Author

Please add a cli Python unit test for this.

Also this should be implemented via a function in the Path helper class.

@firewave , done from my side. Not sure what to do with the failing checks. I've already tried to rerun them, but they again failed.

@chrchr-github chrchr-github merged commit b2511fb into danmar:main Aug 4, 2023
@firewave
Copy link
Collaborator

Turns out we already had the functionality implemented in the FileLister. I will get rid of the redundant code.

@samopolacek samopolacek deleted the build_dir_existence branch August 21, 2023 09:16
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.

3 participants