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

exifprint_lint #1738

Merged
merged 4 commits into from
Jun 25, 2021
Merged

exifprint_lint #1738

merged 4 commits into from
Jun 25, 2021

Conversation

clanmills
Copy link
Collaborator

@clanmills clanmills commented Jun 23, 2021 •
edited
Loading

$ exifprint --lint path

I still have to:

  1. Respect tags which can be Short or Long (such as PixelXDimension)
  2. Add to the test suite.

@clanmills clanmills added this to the v1.00 milestone Jun 23, 2021
@clanmills clanmills self-assigned this Jun 23, 2021
@clanmills clanmills marked this pull request as draft June 23, 2021 18:00
@codecov
Copy link

codecov bot commented Jun 23, 2021 •
edited
Loading

Codecov Report

Merging #1738 (6e638e7) into main (3e099da) will increase coverage by 0.03%.
The diff coverage is 93.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1738      +/-   ##
==========================================
+ Coverage   66.85%   66.89%   +0.03%     
==========================================
  Files         151      151              
  Lines       20729    20754      +25     
==========================================
+ Hits        13858    13883      +25     
  Misses       6871     6871
Impacted Files Coverage Δ
samples/exifprint.cpp 72.22% <93.02%> (+14.77%) ⬆️

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 3e099da...6e638e7. Read the comment docs.

@clanmills clanmills closed this Jun 24, 2021
@clanmills clanmills deleted the exifprint_lint2 branch June 24, 2021 05:51
@clanmills clanmills restored the exifprint_lint2 branch June 24, 2021 05:55
@clanmills clanmills reopened this Jun 24, 2021
@clanmills clanmills requested a review from hassec June 24, 2021 08:49
@clanmills clanmills marked this pull request as ready for review June 24, 2021 08:50
kevinbackhouse
kevinbackhouse previously approved these changes Jun 24, 2021
View reviewed changes
@hassec
Copy link
Member

hassec commented Jun 25, 2021

@clanmills thanks for sharing this feature, cool stuff! 👍

There seems to be a problem with the new test on the WIN platforms though...

@clanmills
Copy link
Collaborator Author

@hassec Thanks for looking at this. I've fixed the python test script to run via cmd.exe on Windows and via bash on other platforms.

@clanmills clanmills merged commit fe83c73 into main Jun 25, 2021
@mergify mergify bot deleted the exifprint_lint2 branch June 25, 2021 12:08
hassec
hassec reviewed Jun 25, 2021
View reviewed changes
cmake/compilerFlags.cmake
@@ -33,7 +33,9 @@ if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN
if (COMPILER_IS_GCC OR COMPILER_IS_CLANG)
# This fails under Fedora - MinGW - Gcc 8.3
if (NOT (MINGW OR CYGWIN OR CMAKE_HOST_SOLARIS))
if (NOT APPLE) # Don't know why this isn't working correctly on Apple with M1 processor
Copy link
Member

Choose a reason for hiding this comment

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

@clanmills is this maybe worth tracking in an issue?
Also didn't this now disable -fstack-clash-protection for all Apple machine? e.g. also for the normal x86 platforms?

@nehaljwani nehaljwani mentioned this pull request Apr 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hassec hassec hassec left review comments

@kevinbackhouse kevinbackhouse kevinbackhouse approved these changes

Assignees

@clanmills clanmills

Labels
samples
Projects
None yet
Milestone
v0.28.0
Development

Successfully merging this pull request may close these issues.
3 participants
@clanmills @hassec @kevinbackhouse