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

Add a patch to insert a static cast to the argument of std::abs #4313

Merged
merged 2 commits into from
Jan 22, 2021
Merged

Add a patch to insert a static cast to the argument of std::abs #4313

merged 2 commits into from
Jan 22, 2021

Conversation

datalogics-robb
Copy link
Contributor

Add a patch to insert a static cast to the argument of std::abs to make IBM's xlclang++ happy.

Fixes issue #4312

Specify library name and version: catch2/2.13.4

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot
Copy link
Collaborator

All green in build 1 (b8f599c99d1b49e486379a1c7368b4949dd74786)! 😊

@github-actions github-actions bot mentioned this pull request Jan 19, 2021
Open
@conan-center-bot
Copy link
Collaborator

All green in build 2 (aea0828837f8a1d1e1ab336e4490628cd8c4d11c)! 😊

@prince-chrismc
Copy link
Contributor

prince-chrismc commented Jan 20, 2021 •
edited
Loading

As per #3903 I think we should wait to see if upstream accepts the patch before merging... this way it reduces our overhead.

catchorg/Catch2#2155

This issue spans a lot of versions which the same patch could be used as well... catchorg/Catch2@4bd2c3a
Was released in 2.10.0 Seems a little superficial to apply to just the last release here

@uilianries uilianries self-requested a review January 20, 2021 12:05
@uilianries
Copy link
Member

Usually I'm not opposite of building patches, but @prince-chrismc comments makes sense. Catch2 releases new versions frequently and I think we could wait for a next one if they have a close date.

@conan-center-bot
Copy link
Collaborator

All green in build 3 (aea0828837f8a1d1e1ab336e4490628cd8c4d11c)! 😊

@datalogics-robb
Copy link
Contributor Author

@prince-chrismc I agree, getting the patch merged upstream is important, and reduces CCI overhead. However, it will not fix the current version, which is why I also produced this PR. It's such a trivial change and clearly follows the patching guidelines. I could have patched older versions, but if a user needs this patch they can update to 2.13.4, so it didn't seem worthwhile. Would you like that done?

@prince-chrismc
Copy link
Contributor

This is what I wanted to see

horenmar merged commit 68975e3 into catchorg:v2.x 36 minutes ago
1 of 2 checks passed

This is absolute a build patch so it's acceptable, but I am no expert to say it's trivial which is why I wanted to delay to upstreams reaction.

@conan-center-bot conan-center-bot merged commit 62f78e2 into conan-io:master Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@intelligide intelligide intelligide approved these changes

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@uilianries uilianries Awaiting requested review from uilianries

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

Successfully merging this pull request may close these issues.
5 participants
@datalogics-robb @conan-center-bot @prince-chrismc @uilianries @intelligide