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

LUAFDN-1756 Manual deserialization of toml into toolspec struct #79

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

afujiwara-roblox
Copy link
Collaborator

Making ToolSpec more generic to support other sources (specifically Artifactory) by changing it from an enum into a struct. We can't rely on Serde to deserialize the toml into a struct, so we need to deserialize it ourselves

@github-actions
Copy link

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, just some minor comments

Host::new("https://gitlab.com".to_string(), Protocol::Gitlab),
),
])
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing purposes, you could probably just declare this statically instead of making a function for it.

src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
src/config.rs Outdated Show resolved Hide resolved
@afujiwara-roblox afujiwara-roblox merged commit 3b14a12 into main Sep 12, 2023
@afujiwara-roblox afujiwara-roblox deleted the LUAFDN-1756 branch September 12, 2023 17:26
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants