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.

Sign up for GitHub

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

Jump to bottom

Use C++ filesystem library/iostreams, added unit tests (#911) #934

Merged
merged 8 commits into from
Oct 25, 2022
Merged

Use C++ filesystem library/iostreams, added unit tests (#911) #934

merged 8 commits into from
Oct 25, 2022

Conversation

uweseimet
Copy link
Contributor

@uweseimet uweseimet commented Oct 23, 2022 •
edited
Loading

Summary of changes:

  • Use C++ filesystem library and C++ iostreams for almost all file operations (gcc < 10.3.0 from buster prevents a complete replacement)
  • Removed obsolete Filepath class
  • Removed duplicate code
  • Renamed fileio.cpp/h to rasdump_fileio.cpp/h, because only rasump still uses this class
  • Fixed more SonarQube issues
  • Added unit tests, total test count is 264

@uweseimet uweseimet linked an issue Oct 23, 2022 that may be closed by this pull request
Use C++ filesystem library/iostreams for file operations #911
Closed
@uweseimet uweseimet marked this pull request as ready for review October 23, 2022 20:21
akuker
akuker approved these changes Oct 25, 2022
View reviewed changes
Copy link
Member

@akuker akuker left a comment

Choose a reason for hiding this comment

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

Approved! Merge away!

@uweseimet
Copy link
Contributor Author

@akuker Please note that I applied two minor changes after your approval and before merging, because the merge result was causing some issues. I hope (and assume) you don't mind these changes:

  1. clang++ still did not compile in my environment because of the LUN_MAX issue I mentioned in your PR. I fixed that.
  2. I added an #ifdef in rascsi.cpp to prevent "SBC_Version::Init();" to be compiled on a standard Linux PC. We have one or two similar #ifdefs for that purpose elsewhere. For running/testing anything where you do not need the RaSCSI hardware for (that's the majority of the code) it is important that you can launch rascsi and execute remote commands on a regular PC, where an IDE like Eclipse is available.

In my environment everything compiles with clang++ without any issues now.

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@uweseimet uweseimet merged commit 6bbaa95 into develop Oct 25, 2022
@uweseimet uweseimet deleted the feature_use_c++_io branch October 25, 2022 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akuker akuker akuker approved these changes

@erichelgeson erichelgeson Awaiting requested review from erichelgeson

@rdmark rdmark Awaiting requested review from rdmark rdmark is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use C++ filesystem library/iostreams for file operations
2 participants
@uweseimet @akuker