Skip to content

aeaconf2 integration#178

Open
safinsingh wants to merge 1 commit into
masterfrom
aeaconf
Open

aeaconf2 integration#178
safinsingh wants to merge 1 commit into
masterfrom
aeaconf

Conversation

@safinsingh

Copy link
Copy Markdown
Collaborator

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.

@safinsingh safinsingh requested a review from sourque October 30, 2023 06:33

@sourque sourque left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread checks.go
Pass []cond
PassOverride []cond
}
var funcRegistry map[string]reflect.Type

Copy link
Copy Markdown
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

Comment thread configs.go
return err
}
field.SetString(datum)
} else if fieldType.Implements(reflect.TypeOf((*aeaconf2.Condition)(nil)).Elem()) {

Copy link
Copy Markdown
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
Copy Markdown
Collaborator 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
Copy Markdown
Collaborator 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.

2 participants