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

Cleanup preset configuration #287

Closed
wants to merge 24 commits into from
Closed

Cleanup preset configuration #287

wants to merge 24 commits into from

Conversation

sarahwooders
Copy link
Collaborator

No description provided.

@sarahwooders sarahwooders requested a review from cpacker November 3, 2023 18:47


@dataclass
class DefaultPreset(Preset):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this refactor is generally the right idea, however I wonder what the best way is to support users adding their own presets, eg: #282

I feel like adding presets should ideally be as easy as adding personas/humans or should have a similar workflow, where you create a text file and put it in a folder.

Thoughts?

@cpacker cpacker mentioned this pull request Nov 7, 2023
Copy link
Collaborator

@cpacker cpacker left a comment

Choose a reason for hiding this comment

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

A lot of the code here look good (eg stripping preset to just take config as input), but we should hold until we refactor the preset creation based on #339, then rebase and merge this on top of the refactor (or the other way - open a PR with the refactor and merge this into the PR)

@cpacker
Copy link
Collaborator

cpacker commented Dec 4, 2023

Closing b/c stale after presets refactor, though we may at some point want to add ideas from this PR into the existing refactor (eg adding a "pretty name" field to the preset YAML files).

@cpacker cpacker closed this Dec 4, 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.

2 participants