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

Implement consistent registry access for vcpkg command accessing ports #517

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

theblackunknown
Copy link

Description

This PR implements the behavior to consistently access a port and its versions for vcpkg command discussed in microsoft/vcpkg#18967

I consider this PR as a draft for now as I have only testing code changes on x-add-version and a new command format-port like it was suggested in the initial PR, and I am looking for feedbacks whether this is going into the right direction or not.

Although I have implement this wanted behavior as a separate struct CommandRegistryPaths containing vcpkg::Path, I feel like this would be more coherent if implemented directly as additional method on vcpkg::RegistryImplementation and vcpkg::RegistryEntry.
However as this is my first PR I lack context and experience with the code base to decide this is a good direction or not.

NOTE: This is only a partial implementation over the initial spec (e.g. read-only registry not implemented).

@theblackunknown
Copy link
Author

@ras0219-msft I take your emoji as a good sign, I'll keep this work going into the same direction !
Please share any feeedbacks whether this would be better to augment RegistryXXX classes instead.

@theblackunknown theblackunknown force-pushed the consistent-registry-access branch from 206f011 to 5ffa68c Compare May 9, 2022 08:46
@theblackunknown theblackunknown force-pushed the consistent-registry-access branch from 5ffa68c to 0da3797 Compare May 18, 2022 21:59
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.

2 participants