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

Print "@version" when talking about port versions more frequently. #1292

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

BillyONeal
Copy link
Member

In microsoft/vcpkg#35164 there was some confusion where a customer thought vcpkg ci was building an old version of openvino because we only printed the version to build for the --dry-run prepass and didn't print the version to build for the actual build. The customer thought we were building the old version rather than the new version as a result.

This change ensures we print the version we are discussing after "Installing" or "Building" and consistently uses the "packagespec@version" form in all the places. For consistency the "packagespec -> version" form we used in a few places is replaced with the @ form.

In microsoft/vcpkg#35164 there was some confusion where a customer thought `vcpkg ci` was building an old version of openvino because we only printed the version to build for the `--dry-run` prepass and didn't print the version to build for the actual build. The customer thought we were building the old version rather than the new version as a result.

This change ensures we print the version we are discussing after "Installing" or "Building" and consistently uses the "packagespec@version" form in all the places. For consistency the "packagespec -> version" form we used in a few places is replaced with the @ form.
@dg0yt
Copy link
Contributor

dg0yt commented Nov 30, 2023

The customer thought we were building the old version rather than the new version as a result.

The customer vcpkg team members ...

@dg0yt
Copy link
Contributor

dg0yt commented Nov 30, 2023

With regard to CI, it is probably still more important to omit unnecessary output for --dry-run (like initially submitted).
Maybe even move the dry-run to a separate step?

@BillyONeal
Copy link
Member Author

With regard to CI, it is probably still more important to omit unnecessary output for --dry-run (like initially submitted).

Since that change was going to require touching docs and stuff here I was lazy and just made us consistently print the version. I don't have strong opinions on next steps; perhaps --parent-hashes should imply 'parent hashes is all I am doing' and make switches like --dry-run have no effect. And possibly --dry-run just shouldn't exist for ci in the first place... I suspect that the only reason it is there is copy pasta from install once upon a time.

In the future I would like to make the ci command smarter by examining what the user actually modifies now that I understand git well enough to write this, but it's not going to be a priority anytime soon.

@BillyONeal BillyONeal merged commit bfb9896 into microsoft:main Dec 4, 2023
5 checks passed
@BillyONeal BillyONeal deleted the add-version-output branch December 4, 2023 23:06
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Dec 5, 2023
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.

3 participants