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

Automatically create alias targets in cmake - Submodule update [ESD-2467] #1199

Merged
merged 8 commits into from Aug 15, 2022
Merged

Automatically create alias targets in cmake - Submodule update [ESD-2467] #1199

merged 8 commits into from Aug 15, 2022

Conversation

lucasle-sn
Copy link
Contributor

@lucasle-sn lucasle-sn commented Jul 28, 2022 •
edited

Jira ticket

https://swift-nav.atlassian.net/browse/ESD-2467

Design

  • The main content for auto ALIAS creation is in cmake. This PR only removes manual ALIAS addition for sbp.

Related PRs

@lucasle-sn lucasle-sn requested review from a team and silverjam as code owners July 28, 2022 06:19
@lucasle-sn lucasle-sn marked this pull request as draft July 28, 2022 06:19
@lucasle-sn lucasle-sn mentioned this pull request Jul 28, 2022
Merged
@silverjam silverjam requested a review from isaactorz July 28, 2022 21:04
isaactorz
isaactorz reviewed Jul 28, 2022
View reviewed changes
Copy link
Contributor

@isaactorz isaactorz left a comment •
edited

Choose a reason for hiding this comment

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

This just a test PR?

Looks good regardless. I think we should be ok regardless of which of these changes lands first as they percolate up the submodule tree (cmake or the one here).

@lucasle-sn
Copy link
Contributor Author

lucasle-sn commented Jul 28, 2022 •
edited

This just a test PR?

Looks good regardless. I think we should be ok regardless of which of these changes lands first as they percolate up the submodule tree (cmake or the one here).

Hi @isaactorz, the main objective of this PR is just to check the new update in cmake swift-nav/cmake#124. It will automatically create alias for all swift targets, under naming convention swiftnav::${target}. So the line add_library(swiftnav::sbp ALIAS sbp) becomes redundant. But it will not cause any issue.

@lucasle-sn lucasle-sn marked this pull request as ready for review July 29, 2022 00:41
silverjam
silverjam reviewed Aug 1, 2022
View reviewed changes
Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

@lucasle-sn removing from my review queue, retag if necessary

@lucasle-sn lucasle-sn merged commit c41b5b8 into master Aug 15, 2022
@lucasle-sn lucasle-sn deleted the lucasle/esd-2467/AutomaticallyCreateAliasTargetsInCmake branch August 15, 2022 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silverjam silverjam silverjam left review comments

@isaactorz isaactorz isaactorz left review comments

@RReichert RReichert RReichert approved these changes

@antrikshsrivastava antrikshsrivastava antrikshsrivastava approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@lucasle-sn @silverjam @RReichert @antrikshsrivastava @isaactorz