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

Refactor PCL_MAKE_PKGCONFIG #2894

Conversation

SunBlack
Copy link
Contributor

@SunBlack SunBlack commented Mar 6, 2019

Merge PCL_MAKE_PKGCONFIG_HEADER_ONLY with PCL_MAKE_PKGCONFIG. PCL_MAKE_PKGCONFIG is now using named arguments. This should increase readability of this function (was always wondering about "" "" "" "" "" at end of macro call).

I run at end git diff --no-index pcl_install/lib/pkgconfig/ pkgconfig_old/ >> diff to be sure nothing changed.

Not sure about naming of parameter if we should use e.g. DESC or DESCRIPTION or PCL_DEPS or SUBSYS_DEPS.

Copy link
Member

@taketwo taketwo left a comment

Choose a reason for hiding this comment

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

I'm fine with the names you gave to the parameters.

cmake/pcl_targets.cmake Outdated Show resolved Hide resolved
Co-Authored-By: SunBlack <SunBlack@users.noreply.github.com>
Copy link
Member

@taketwo taketwo left a comment

Choose a reason for hiding this comment

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

Thanks, it's so much better now!

Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

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

👍 Very neat. Thanks

@SergioRAgostinho SergioRAgostinho merged commit 1ec44f0 into PointCloudLibrary:master Mar 6, 2019
@SunBlack SunBlack deleted the refactor_PCL_MAKE_PKGCONFIG branch March 6, 2019 22:00
@taketwo taketwo changed the title Refactor PCL_MAKE_PKGCONFIG Refactor PCL_MAKE_PKGCONFIG Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants