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

Proposed fix for pystring include paths #1900

Conversation

irieger
Copy link

@irieger irieger commented Nov 6, 2023

Make Findpystring.cmake return the full include directory to pystring so that prefixing includes with "pystring/" is no longer required.

This changes the behaviour of the included Findpystring.cmake to match the bahaviour of other regular find_package modules to report the full include directory removing the requirement to use "pystring/" when including "pystring.h" etc.
(This follows from conan-io/conan-center-index#20703 (comment))

Make Findpystring.cmake return the full include directory to pystring so that prefixing includes with "pystring/"
is no longer required.

Signed-off-by: Ingmar Rieger <[email protected]>
Copy link

CLA Not Signed

@remia
Copy link
Collaborator

remia commented Nov 6, 2023

Thanks @irieger, FYI there was a related discussion today in #1897, here is a similar commit I did earlier.

@irieger
Copy link
Author

irieger commented Nov 7, 2023

Good to see. Will close this and comment with some feedback on your Pull request.

@irieger irieger closed this Nov 7, 2023
@irieger
Copy link
Author

irieger commented Nov 7, 2023

Added a comment to remia#52 as the current state wouldn't build on Arch Linux but otherwise it contains the changes as proposed here.

@remia
Copy link
Collaborator

remia commented Nov 7, 2023

Hi @irieger,

I don't see your comment, can you double check you published it? Thanks for looking into it.

@irieger
Copy link
Author

irieger commented Nov 7, 2023

New to those github workflows. Just sent the review. Hadn't realised I need to push another button. Gitlab which is my day to day tool has a much more prominent submit button ...

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