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

aeaconf2 integration #178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

aeaconf2 integration #178

wants to merge 1 commit into from

Conversation

safinsingh
Copy link
Member

Tracking issue for new config format: https://github.com/safinsingh/aeaconf2

This initial commit contains some basic work for integrating aeaconf2. It does not compile yet. Transitioning should be fairly easy based on what I've done already. Please let me know if I need to make any changes to aeaconf2 to accommodate for aeacus's functionality.

Copy link
Collaborator

@sourque sourque left a comment

Choose a reason for hiding this comment

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

Looks really cool. Should be a huge QoL improvement for writing configs.

Pass []cond
PassOverride []cond
}
var funcRegistry map[string]reflect.Type
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can init this in var with type inference eg var funcRegistry = make(map[string]reflect.Type). Having a map to a reflect type smells really bad though LOL I'll have to understand the code

return err
}
field.SetString(datum)
} else if fieldType.Implements(reflect.TypeOf((*aeaconf2.Condition)(nil)).Elem()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may look better as a function on the aeaconf struct. Maybe something like ModifyFields that takes a func(string) string as a parameter? Then we don't have to do the Implements stuff

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I could probably just do that on the aeaconf side

Copy link
Member Author

Choose a reason for hiding this comment

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

Added on v0.1.2 (compat.ModifyConditionStrings)

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.

None yet

2 participants