feat(config): use toml for configuring zizmor#1666
Draft
reubenwong97 wants to merge 1 commit into
Draft
Conversation
Contributor
Author
|
Hi @woodruffw , I have a first draft of the change to use TOML configs here. Could you help to take a quick look when you have time? I'm not the most confident in these changes, primarily due to introducing an extra Thanks a lot! Best, |
Member
|
Thanks @reubenwong97, I'll try and give this a review in the coming days! My time for the next few days is somewhat limited, so it might take me a bit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-submission checks
Please check these boxes:
Mandatory: This PR corresponds to an issue: Switch to TOML for zizmor's config #1560 .
I hereby disclose the use of an LLM or other AI coding assistant in the
creation of this PR. PRs will not be rejected for using AI tools, but
will be rejected for undisclosed use.
If a checkbox is not applicable, you can leave it unchecked.
Summary
This PR is currently a DRAFT for the purposes of obtaining feedback.
The changes are broadly:
ConfigFormatenum so that the appropriate deserializer is called.loadto acceptConfigFormatserde_json::Valueas the intermediate representation. This was chosen since its variants could be produced by yaml or toml configs in zizmor (as opposed toserde_yaml::Valuewhich hasTaggedvariants for example).ForbiddenUsesConfigInner, the change toserde_jsonremoved keys being deserialized as yaml tags from being an issue.ConfigErrorInner::Syntaxsince it could be due to a toml or serde_yaml deserialization error.TODO:
[ ] Migration from yaml config to toml
Test Plan
I've so far tested dummy configs (yaml and toml) against the coreutils workflows and managed to obtain the same results. When the approach is more set in stone, I will add some tests.