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

Refactoring & cleanup #2109

Merged
merged 25 commits into from
Feb 21, 2022
Merged

Refactoring & cleanup #2109

merged 25 commits into from
Feb 21, 2022

Conversation

piponazo
Copy link
Collaborator

Here it goes a bunch of totally unrelated changes with the aim of simplifying code and removing some dead code (take a look to the LoC deletions 😨 ). In case it is difficult to review so many changes, I can split the PR in few ones by different topics. Although I tried to keep different changes in isolated commits.

This is a list of the different actions taken here:

  • Usage of string_view in some more places.
  • Usage of filesystem in some more places.
  • Deletion of dead code.
  • Cleanup header inclusions.
  • Address some issues discovered with cppcheck.

@piponazo piponazo added the refactoring Cleanup / Simplify code -> more readable / robust label Feb 18, 2022
@piponazo piponazo self-assigned this Feb 18, 2022
@@ -278,6 +284,7 @@ namespace Exiv2 {
return Image::byteSwap(v,bSwap);
}

/// \todo not used internally. At least we should test it
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added these comments in few places because cppcheck was detecting these functions as UNUSED ones. However they are part of the API and they should not be removed without further consideration.

@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #2109 (21eb0ce) into main (0b17cc3) will increase coverage by 0.18%.
The diff coverage is 34.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2109      +/-   ##
==========================================
+ Coverage   63.80%   63.98%   +0.18%     
==========================================
  Files          96       97       +1     
  Lines       19169    19086      -83     
  Branches     9772     9752      -20     
==========================================
- Hits        12230    12212      -18     
+ Misses       4658     4599      -59     
+ Partials     2281     2275       -6     
Impacted Files Coverage Δ
app/actions.hpp 100.00% <ø> (ø)
app/exiv2app.hpp 100.00% <ø> (ø)
include/exiv2/basicio.hpp 100.00% <ø> (ø)
include/exiv2/bmffimage.hpp 100.00% <ø> (ø)
include/exiv2/datasets.hpp 100.00% <ø> (ø)
include/exiv2/error.hpp 65.51% <ø> (ø)
include/exiv2/image.hpp 100.00% <ø> (ø)
include/exiv2/tags.hpp 100.00% <ø> (ø)
include/exiv2/types.hpp 70.58% <ø> (ø)
include/exiv2/value.hpp 86.56% <ø> (ø)
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b17cc3...21eb0ce. Read the comment docs.

src/makernote_int.cpp Outdated Show resolved Hide resolved
include/exiv2/futils.hpp Outdated Show resolved Hide resolved
src/basicio.cpp Outdated Show resolved Hide resolved
include/exiv2/bmffimage.hpp Outdated Show resolved Hide resolved
src/futils.cpp Outdated Show resolved Hide resolved
@piponazo piponazo merged commit c5a9dfd into main Feb 21, 2022
@piponazo piponazo deleted the main_StringView branch February 21, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleanup / Simplify code -> more readable / robust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants