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

Add registry configuration for Version Settings. #2907

Merged
merged 8 commits into from
Jul 29, 2022
Merged

Conversation

anrossi
Copy link
Contributor

@anrossi anrossi commented Jul 23, 2022

Description

Enable configuration of Version Settings via the Windows Registry.

Testing

Manual testing with quicinteropserver and quicinterop.

  • Automated tests should be written to cover the storage system.

Documentation

  • Documentation should be updated to reflect the new Registry value names.

@anrossi anrossi requested a review from a team as a code owner July 23, 2022 01:40
@anrossi
Copy link
Contributor Author

anrossi commented Jul 24, 2022

@nibanks Currently, this expects the registry format for these to be binary blobs, which is difficult to use since .reg files apparently don't support the REG_BINARY type. It's easy to read into memory, however.
Should we instead use something like REG_MULTISZ or a regular string with comma-separated values (in hex) which is easier for admins, but requires more work from us when reading in (and the risks of string parsing).
I'm open to design suggestions.

src/core/settings.c Outdated Show resolved Hide resolved
@nibanks
Copy link
Member

nibanks commented Jul 24, 2022

@nibanks Currently, this expects the registry format for these to be binary blobs, which is difficult to use since .reg files apparently don't support the REG_BINARY type. It's easy to read into memory, however. Should we instead use something like REG_MULTISZ or a regular string with comma-separated values (in hex) which is easier for admins, but requires more work from us when reading in (and the risks of string parsing). I'm open to design suggestions.

I thought REG_BINARY is supported everywhere. Try exporting one to a .reg file to see if it works.

@nibanks
Copy link
Member

nibanks commented Jul 24, 2022

Looks like CLOG failed. Please update the sidecar.

@nibanks nibanks added the Area: Core Related to the shared, core protocol logic label Jul 24, 2022
@anrossi
Copy link
Contributor Author

anrossi commented Jul 25, 2022

@nibanks Currently, this expects the registry format for these to be binary blobs, which is difficult to use since .reg files apparently don't support the REG_BINARY type. It's easy to read into memory, however. Should we instead use something like REG_MULTISZ or a regular string with comma-separated values (in hex) which is easier for admins, but requires more work from us when reading in (and the risks of string parsing). I'm open to design suggestions.
I thought REG_BINARY is supported everywhere. Try exporting one to a .reg file to see if it works.

Ah, I tried editing the .reg file in notepad and it garbled the file encoding causing the error I was seeing; when I used VSCode to edit the .reg file, it was easy to import. I'll keep the REG_BINARY type unless you think there's a better solution.

src/test/lib/ApiTest.cpp Outdated Show resolved Hide resolved
@nibanks nibanks merged commit a8f3325 into main Jul 29, 2022
@nibanks nibanks deleted the anrossi/vn-regkeys branch July 29, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core Related to the shared, core protocol logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants