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

meson CI additions #2505

Merged
merged 22 commits into from
Feb 11, 2023
Merged

meson CI additions #2505

merged 22 commits into from
Feb 11, 2023

Conversation

neheb
Copy link
Collaborator

@neheb neheb commented Feb 10, 2023

No description provided.

@ghost
Copy link

ghost commented Feb 10, 2023

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

@neheb neheb force-pushed the meson branch 3 times, most recently from 5fc8b0e to db1c398 Compare February 10, 2023 01:24
@neheb
Copy link
Collaborator Author

neheb commented Feb 10, 2023

../src/tiffimage_int.cpp(1957): fatal error C1001: An internal error has occurred in the compiler.

...interesting

@neheb neheb force-pushed the meson branch 4 times, most recently from 60e15b3 to b54b5f8 Compare February 10, 2023 02:14
@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #2505 (e333386) into main (b8b4a04) will decrease coverage by 0.22%.
The diff coverage is 76.74%.

❗ Current head e333386 differs from pull request most recent head 046cb91. Consider uploading reports for the commit 046cb91 to get more accurate results

@@            Coverage Diff             @@
##             main    #2505      +/-   ##
==========================================
- Coverage   64.67%   64.45%   -0.22%     
==========================================
  Files         104      104              
  Lines       22260    22385     +125     
  Branches    10848    10926      +78     
==========================================
+ Hits        14397    14429      +32     
- Misses       5622     5707      +85     
- Partials     2241     2249       +8     
Impacted Files Coverage Δ
app/getopt.hpp 100.00% <ø> (ø)
include/exiv2/value.hpp 85.52% <ø> (ø)
src/asfvideo.cpp 53.26% <ø> (-2.87%) ⬇️
src/canonmn_int.cpp 73.48% <ø> (ø)
src/crwimage_int.hpp 93.75% <ø> (ø)
src/matroskavideo.cpp 4.10% <0.00%> (-0.43%) ⬇️
src/nikonmn_int.cpp 63.11% <ø> (ø)
src/olympusmn_int.cpp 44.68% <ø> (ø)
src/tags_int.hpp 91.34% <ø> (ø)
src/tiffvisitor_int.cpp 81.20% <ø> (-0.16%) ⬇️
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@neheb neheb force-pushed the meson branch 4 times, most recently from 4081967 to c24f829 Compare February 10, 2023 05:25
@neheb neheb marked this pull request as ready for review February 10, 2023 19:48
@neheb neheb force-pushed the meson branch 9 times, most recently from 2301003 to e333386 Compare February 11, 2023 05:37
These are the wrong way around. MSVC warns with /W4.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Fixes MSVC warning.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Fixes MSVC's warning C4706: assignment within conditional expression

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Fixes warning C4702: unreachable code

No other way to make all compilers gappy.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Suggested by MSVC.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Fixes: bugprone-integer-division

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Found with bugprone-assignment-in-if-condition

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Found with bugprone-not-null-terminated-result

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
fixes internal MSVC libc++ warnings

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Forgotten when adding xmp support.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Useful for CI.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
A rebasing error got rid of this.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
if (data2_ < other.data2_)
return true;
else if (data2_ == other.data2_) {
if (data2_ == other.data2_) {
if (data3_ < other.data3_)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you mind to take this opportunity to add the brackets were they are missing? It looks super confusing to have some ifs/elses with them and other without.

return {};
if (metadataId == mdExif)
return r->exifSupport_;
if (metadataId == mdIptc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] I see why you are making this change (I have been there and done this 😛 ) . However, would it not make more sense to have else if statements after the first if ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really. else after return is redundant.

@piponazo
Copy link
Collaborator

I see that many of the changes here are also present in #2508. Is this intended or was it an accident? Which PR do you plan to merge first?

@neheb
Copy link
Collaborator Author

neheb commented Feb 11, 2023

I'll merge this one. The other contains fixes exposed by enabling W4 on MSVC which I then rebased this on it too see how many warnings went away. I'll handle the remaining ones later.

@neheb neheb merged commit 976dcd8 into Exiv2:main Feb 11, 2023
@neheb neheb deleted the meson branch February 11, 2023 18:44
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.

2 participants