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

refactor: Replace deprecated libtiff v4.3 typedefs with C99 fixed-size integers #685

Merged
merged 3 commits into from
Jun 25, 2022
Merged

refactor: Replace deprecated libtiff v4.3 typedefs with C99 fixed-size integers #685

merged 3 commits into from
Jun 25, 2022

Conversation

marco-langer
Copy link
Contributor

@marco-langer marco-langer commented Jun 9, 2022 •
edited by mloskot
Loading

Description

This PR replaces libtiff's fixed-size typedef uint32 with C99's fixed-size typedef std::uint32_t.

Rational: libtiff's typedefs are deprecated since v4.3.0 and we already see this deprecation warning in our CI for builds with clang on macos-10.15.

C99's fixed-size typedefs and libtiffs typedefs are almost interchangeable, with the notable exception for 64 bit macOS and a breaking change between long and long long according to this and this discussion. Currently I don't think this will affect Gil as we only used uint32 from libtiffs typedefs, but nevertheless it might be a good idea to check the impact of this PR on macOS builds before merging it into develop.

  • Add test case(s)
  • Ensure all CI builds pass
  • Review and approve

@marco-langer marco-langer changed the title replaced deprecated libtiff v4.3 typedefs with C99 fixed width integers replaced deprecated libtiff v4.3 typedefs with C99 fixed-size integers Jun 9, 2022
@codecov
Copy link

codecov bot commented Jun 9, 2022 •
edited
Loading

Codecov Report

Merging #685 (c1026cb) into develop (526c898) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #685   +/-   ##
========================================
  Coverage    80.32%   80.32%           
========================================
  Files          117      117           
  Lines         5032     5032           
========================================
  Hits          4042     4042           
  Misses         990      990

@mloskot mloskot added this to the Boost 1.80 milestone Jun 9, 2022
@mloskot mloskot added the cat/refactoring Any nonfunctional changes label Jun 9, 2022
@mloskot mloskot changed the title replaced deprecated libtiff v4.3 typedefs with C99 fixed-size integers refactor: Replace deprecated libtiff v4.3 typedefs with C99 fixed-size integers Jun 9, 2022
mloskot
mloskot approved these changes Jun 9, 2022
View reviewed changes
Copy link
Member

@mloskot mloskot left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@striezel
Copy link
Contributor

C99's fixed-size typedefs and libtiffs typedefs are almost interchangeable, with the notable exception for 64 bit macOS and a breaking change between long and long long according to this and this discussion. Currently I don't think this will affect Gil as we only used uint32 from libtiffs typedefs, but nevertheless it might be a good idea to check the impact of this PR on macOS builds before merging it into develop.

I don't have much of a clue about macOS in general, but FWIW GitHub Actions is now also offering macOS 12 runners: https://github.blog/changelog/2022-06-13-github-actions-macos-12-for-github-hosted-runners-is-now-generally-available/ Maybe that can be helpful here to extend tests to newer macOS versions.

mloskot added a commit that referenced this pull request Jun 25, 2022
@mloskot
test: Add tiled TIFF test case to simple_all_formats
526c898 
This is to hit more lines and make codecov happier,
than during recent build of PR #685
mloskot
mloskot approved these changes Jun 25, 2022
View reviewed changes
Copy link
Member

@mloskot mloskot left a comment

Choose a reason for hiding this comment

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

All CI's are now green. Thanks again!

@mloskot mloskot merged commit 151fd9c into boostorg:develop Jun 25, 2022
mloskot added a commit that referenced this pull request Jun 27, 2022
@mloskot
Merge branch 'develop'
b29f3da 
* develop:
  docs!: Announce plan to require C++17 after Boost 1.80 (#694)
  feat: Added apply_rasterizer() free function (#695)
  refactor: Ellipse rasterizer according to the comment at (#692)
  refactor: Deprecate apply_operation in favor of variant2::visit for any_image (#656)
  refactor: Replace deprecated libtiff v4.3 typedefs with C99 fixed-size integers (#685)
  fix: Automatic detection of <filesystem> header (#684)
  test: Add tiled TIFF test case to simple_all_formats
  test: Add tests for RGB to HSL (#691)
  refactor: Move RGB to HSL tests to color_convert_rgb.cpp
  refactor: Make with_tolerance reusable across other tests
  chore: Correct include guard
  fix: Add missing #include <array>
  fix: Wrong RGB -> HSL convertion (#505)
@mloskot mloskot mentioned this pull request Jul 5, 2022
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mloskot mloskot mloskot approved these changes

Assignees
No one assigned
Labels
cat/refactoring Any nonfunctional changes
Projects
None yet
Milestone
Boost 1.80
Development

Successfully merging this pull request may close these issues.
3 participants
@marco-langer @striezel @mloskot