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

Fix HEADER_SEARCH_PATHS and EXTRA_LIBRARY_SEARCH_PATHS #727

Merged
merged 3 commits into from
Jul 19, 2022
Merged

Fix HEADER_SEARCH_PATHS and EXTRA_LIBRARY_SEARCH_PATHS #727

merged 3 commits into from
Jul 19, 2022

Conversation

kauwua
Copy link
Contributor

@kauwua kauwua commented Jul 16, 2022

They were relative to the jucer folder instead of project folder

Fixes #726

Copy link
Owner

@McMartin McMartin left a comment

Choose a reason for hiding this comment

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

Thanks a lot for opening this pull request!

I found a few things that I either missed or just didn't think about earlier. Let me know if something is not clear. I'm also happy to push fixup commits if you want.

Jucer2CMake/src/reprojucer.hpp Outdated Show resolved Hide resolved
Jucer2CMake/src/reprojucer.hpp Show resolved Hide resolved
Jucer2CMake/src/reprojucer.hpp Outdated Show resolved Hide resolved
cmake/Reprojucer.cmake Outdated Show resolved Hide resolved
@kauwua
Copy link
Contributor Author

kauwua commented Jul 16, 2022

I updated the PR based on your comments, hope I did not miss too much this time ! the pipeline is failing because of the changes in the generated CMakeLists.txt, but I don't think I can get the artefacts from the pipeline, you probably have to do that

@kauwua
Copy link
Contributor Author

kauwua commented Jul 16, 2022

Btw I did not format the code because it was also changing other parts of the code, the CI does not seem to check this file either

@McMartin
Copy link
Owner

McMartin commented Jul 17, 2022

@Riuzakiii Thanks a lot for addressing my comments quickly!

I pushed several fixup commits to address some more "comments" I would have. I hope you don't mind.

the pipeline is failing because of the changes in the generated CMakeLists.txt,

Indeed, I pushed 74905a8 to address that (please keep it as a separate commit).

the CI does not seem to check this file either

Very good catch ! 👏🏻
I opened #728 to fix it. I'll merge it first, so you'll have to rebase on top of the latest main.

@kauwua
Copy link
Contributor Author

kauwua commented Jul 17, 2022

No problem, I don't mind at all ! I will rebase the branch after you merge #728 and keep the changes of the generated file as a separate commit.

@McMartin
Copy link
Owner

McMartin commented Jul 18, 2022

@Riuzakiii thanks for rebasing! Since I also merged #724 in the meantime, the files under generated/JUCE-7.0.0 also need to be updated. I'll take care of that as soon as possible.

@McMartin McMartin changed the title Fix HEADER_SEARCH_PATHS and EXTRA_LIBRARY_SEARCH_PATHS, they were rel… Fix HEADER_SEARCH_PATHS and EXTRA_LIBRARY_SEARCH_PATHS Jul 18, 2022
@McMartin
Copy link
Owner

@Riuzakiii I rebased one last time to slightly reword the first commit (I did the same thing for the PR title), and I also pushed a final commit to add you to the table of contributors. Please have a look and double check that what I wrote is correct, thanks!

@kauwua
Copy link
Contributor Author

kauwua commented Jul 18, 2022

Looks good to me, thanks for adding me to the table of contributors

@McMartin McMartin merged commit 57bef2c into McMartin:main Jul 19, 2022
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.

Issue with "Header Search Paths" and "Extra Library Search Paths" in exporter
2 participants