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

[starlink-ast] create new port #17105

Merged
merged 25 commits into from Jul 23, 2021
Merged

[starlink-ast] create new port #17105

merged 25 commits into from Jul 23, 2021

Conversation

veggiesaurus
Copy link
Contributor

This PR adds a new port for the widely used Starlink astrometry library AST.

Only Windows x86/x64 is supported. Two optional features (yaml and pthreads) are available.

@veggiesaurus veggiesaurus marked this pull request as draft April 5, 2021 20:01
@autoantwort
Copy link
Contributor

Run ./vcpkg format-manifest ports/starlink-ast/vcpkg.json to make the pipeline happy :)

@NancyLi1013 NancyLi1013 self-assigned this Apr 6, 2021
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 6, 2021
NancyLi1013
NancyLi1013 suggested changes Apr 8, 2021
View reviewed changes
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/vcpkg.json Outdated Show resolved Hide resolved
versions/s-/starlink-ast.json Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 added the depends:upstream-changes Waiting on a change to the upstream project label Apr 13, 2021
@veggiesaurus veggiesaurus marked this pull request as ready for review April 14, 2021 14:54
autoantwort
autoantwort suggested changes Apr 14, 2021
View reviewed changes
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
NancyLi1013
NancyLi1013 suggested changes Apr 15, 2021
View reviewed changes
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
ports/starlink-ast/vcpkg.json Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

Hi @veggiesaurus

Could you please address the review suggestions? Thanks.

@veggiesaurus
Copy link
Contributor Author

Hi @veggiesaurus

Could you please address the review suggestions? Thanks.

Will do so today. Sorry for not sorting it out sooner.

veggiesaurus and others added 7 commits July 20, 2021 12:49
@veggiesaurus @NancyLi1013 @autoantwort
Apply suggestions from code review
5844fa8 
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: autoantwort <41973254+autoantwort@users.noreply.github.com>
@veggiesaurus
adds supports field to json
fabbe01
@veggiesaurus @autoantwort
Update ports/starlink-ast/portfile.cmake
036a337 
adjust configure options

Co-authored-by: autoantwort <41973254+autoantwort@users.noreply.github.com>
@veggiesaurus
Merge branch 'veggiesaurus/starlink_ast' of https://github.com/veggie…
3138435 
…saurus/vcpkg into veggiesaurus/starlink_ast
@veggiesaurus
removed --disabled-shared and MINPACK_NO_DLL
d471bb7
@veggiesaurus
use DETERMINE_BUILD_TRIPLET to determine host
28afdbd
@veggiesaurus
updates git-tree hash
acfa89d
@veggiesaurus
Copy link
Contributor Author

@NancyLi1013 @autoantwort changes have been implemented, please re-review.

(still downloading the tar.gz directly, rather than cloning the repo, as the release tar.gz has additional files)

NancyLi1013
NancyLi1013 suggested changes Jul 21, 2021
View reviewed changes
ports/starlink-ast/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

Have you tested the features pthreads and yaml?

@veggiesaurus
Copy link
Contributor Author

Have you tested the features pthreads and yaml?

yes, I've tested those features as well 👍

@NancyLi1013
Copy link
Contributor

Please run vcpkg x-add-version --overwrite-version starlink-ast to solve the failures on x86-windows.

@NancyLi1013 NancyLi1013 removed the depends:upstream-changes Waiting on a change to the upstream project label Jul 21, 2021
@NancyLi1013
Copy link
Contributor

LGTM now, thanks for your PR. @veggiesaurus

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Jul 21, 2021
@vicroms vicroms merged commit bb82997 into microsoft:master Jul 23, 2021
@vicroms
Copy link
Member

vicroms commented Jul 23, 2021

Thanks for the PR!

@veggiesaurus veggiesaurus deleted the veggiesaurus/starlink_ast branch October 6, 2021 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@autoantwort autoantwort autoantwort requested changes

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@veggiesaurus @autoantwort @NancyLi1013 @vicroms