Skip to content

[docs] Add comment about VCPKG_TARGET_TRIPLET to README and installing package#19626

Closed
SallySoul wants to merge 2 commits intomicrosoft:masterfrom
intact-solutions:cmake_triplet_pr
Closed

[docs] Add comment about VCPKG_TARGET_TRIPLET to README and installing package#19626
SallySoul wants to merge 2 commits intomicrosoft:masterfrom
intact-solutions:cmake_triplet_pr

Conversation

@SallySoul
Copy link

Describe the pull request

  • What does your PR fix?

I recently had an issue where I needed to specify VCPKG_TARGET_TRIPLET. I now see that I missed the critical piece of information at the bottom of integration.md. However, I'm of the opinion. VCPKG_TARGET_TRIPLET deserved some mention in a couple other areas.

  • Which triplets are supported/not supported? Have you updated the [CI baseline]

Its a documentation update.

  • Does your PR follow the [maintainer guide]

To the best of my knowledge

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

N/A

@ghost
Copy link

ghost commented Aug 17, 2021

CLA assistant check
All CLA requirements met.

@PhoebeHui PhoebeHui changed the title Add comment about VCPKG_TARGET_TRIPLET to README and installing package [docs] Add comment about VCPKG_TARGET_TRIPLET to README and installing package Aug 18, 2021
@PhoebeHui PhoebeHui added the category:documentation To resolve the issue, documentation will need to be updated label Aug 18, 2021
@PhoebeHui
Copy link
Contributor

@SallySoul, thanks for the PR! Could you sign the CLA?

@SallySoul
Copy link
Author

Signed. I can't re-run the checks though.

@PhoebeHui
Copy link
Contributor

The baseline issue would be fixed by PR #19633. I will rerun the CI after the PR merged.

@strega-nil-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY requested a review from PhoebeHui August 23, 2021 02:19
@PhoebeHui PhoebeHui added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Aug 23, 2021
@Neumann-A
Copy link
Contributor

also needs info about VCPKG_HOST_TRIPLET to be complete

@SallySoul
Copy link
Author

@Neumann-A sure, I'll update the PR shortly.

@SallySoul
Copy link
Author

@PhoebeHui Is there anything needed from me to push this along?

@PhoebeHui
Copy link
Contributor

@SallySoul, It looks good for me now, thanks!

@BillyONeal
Copy link
Member

I'm not entirely convinced that this warrants such a big block in the front "readme" -- since the longer readmes get the less likely people are to actually read them, and we don't expect most customers to need to customize triplets that often. If it is something that should be in the readme, I made some changes I would prefer to see against this: BillyONeal@d17fdc0

I want @strega-nil / @strega-nil-ms to take a look before landing this since she has Strong opinions about our docs.

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Sep 3, 2021
@strega-nil-ms
Copy link
Contributor

I'm of the opinion that triplets are important enough that they belong in the README, or at least in the first thing people read; however, it doesn't make sense to me to put it under the CMake integration section. Maybe it's own section?

@BillyONeal BillyONeal removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Sep 30, 2021
@BillyONeal
Copy link
Member

The maintainers talked about this in person today and we are OK with adding a block about triplets into the readme but would like @SallySoul to address Nicole's (@strega-nil / @strega-nil-ms ) concerns before merging this.

@SallySoul
Copy link
Author

@strega-nil Happy to make changes. However, I need some clarification.

First, my experience with vcpkg is limited to use with cmake projects. What are some other use cases of vcpkg targets that I should be mindful of?

Also, where would you like this new section in the README? Given that cmake has a relatively large block in the the current structure, it's not clear to me where a triplet section would belong. Maybe right after the cmake section under "Getting Started"?

@strega-nil-ms
Copy link
Contributor

@SallySoul I honestly think that yeah, it could just be its own section. Adding a blurb about how to use this from msbuild would be good too, but I can write that blurb.

@PhoebeHui
Copy link
Contributor

Ping @SallySoul for response.

@PhoebeHui
Copy link
Contributor

We don't have permission to push changes to this PR, closing this PR since it seems that no progress is being made. Please reopen if work is still being done.

@PhoebeHui PhoebeHui closed this Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:documentation To resolve the issue, documentation will need to be updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants