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

Add more tests to make sure the package does not regress. #7

Merged
merged 20 commits into from
Mar 20, 2024

Conversation

vgvassilev
Copy link
Contributor

@vgvassilev vgvassilev commented Mar 17, 2024

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@vgvassilev vgvassilev force-pushed the add-tests branch 13 times, most recently from 83b3bf7 to a23e2cf Compare March 18, 2024 21:44
@vgvassilev
Copy link
Contributor Author

@mcbarton, @fsfod, we seem to have a similar problem to the one we fixed in compiler-research/CppInterOp#196. I see that some of the exports are dependent on the MSVC version and I was wondering if we can carry these exports somehow in the CppInterOpConfig.cmake. Alternatively, we could use a sledgehammer like CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS but I am not sure if that's a good idea. Any thoughts?

@vgvassilev
Copy link
Contributor Author

@conda-forge-admin, please rerender

@vgvassilev vgvassilev force-pushed the add-tests branch 4 times, most recently from ccb1f26 to d308ba2 Compare March 20, 2024 07:55
@mcbarton
Copy link
Contributor

@mcbarton, @fsfod, we seem to have a similar problem to the one we fixed in compiler-research/CppInterOp#196. I see that some of the exports are dependent on the MSVC version and I was wondering if we can carry these exports somehow in the CppInterOpConfig.cmake. Alternatively, we could use a sledgehammer like CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS but I am not sure if that's a good idea. Any thoughts?

@vgvassilev The export symbols method is known to be brittle to the version of MSVC used compiler-research/CppInterOp#188 (comment) . @fstod discusses his opinion on WINDOWS_EXPORT_ALL_SYMBOLS in this message, suggestion he doesn't think it will work. He also suggests a longer term solution than these methods.

@vgvassilev
Copy link
Contributor Author

@mcbarton, @fsfod, we seem to have a similar problem to the one we fixed in compiler-research/CppInterOp#196. I see that some of the exports are dependent on the MSVC version and I was wondering if we can carry these exports somehow in the CppInterOpConfig.cmake. Alternatively, we could use a sledgehammer like CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS but I am not sure if that's a good idea. Any thoughts?

@vgvassilev The export symbols method is known to be brittle to the version of MSVC used compiler-research/CppInterOp#188 (comment) . @fstod discusses his opinion on WINDOWS_EXPORT_ALL_SYMBOLS in this message, suggestion he doesn't think it will work. He also suggests a longer term solution than these methods.

Yes. Understood. I believe compiler-research/CppInterOp#213 fixes my problem for now...

@vgvassilev
Copy link
Contributor Author

@conda-forge-admin, please rerender

@vgvassilev vgvassilev merged commit 4ac7491 into conda-forge:main Mar 20, 2024
5 checks passed
@vgvassilev vgvassilev deleted the add-tests branch March 20, 2024 16:56
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.

3 participants