Combine config into single config prop #71
Closed
Conversation
This file contains 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
cookpete
force-pushed
the
single-config-prop
branch
from
db1730d to
9dd7ab3
Compare
cookpete
force-pushed
the
single-config-prop
branch
from
9dd7ab3 to
c81b7f2
Compare
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.
I don't like how we have to carry around four separate
configprops in several places, especially for props that I think aren't used that often. This consolidates all the players' configuration data into oneconfigprop, which falls back to adefaultConfigobject for any property that isn't specified.Still not sure if this is better than the current way of doing things. I'm happy to hear feedback if anyone has any.
This is definitely a breaking change and perhaps should wait until we're ready for a
v1.0release.