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

[DEV] Create new configuration management #347

Closed
sgdc3 opened this issue Dec 14, 2015 · 12 comments
Closed

[DEV] Create new configuration management #347

sgdc3 opened this issue Dec 14, 2015 · 12 comments
Milestone

Comments

@sgdc3
Copy link
Member

sgdc3 commented Dec 14, 2015

No description provided.

@sgdc3 sgdc3 added this to the 5.2 Relase milestone Dec 14, 2015
@sgdc3
Copy link
Member Author

sgdc3 commented Dec 14, 2015

#90

@sgdc3
Copy link
Member Author

sgdc3 commented Dec 14, 2015

#311

@ljacqu
Copy link
Member

ljacqu commented Dec 18, 2015

Who is doing this? This task is blocking a lot of other issues

@sgdc3
Copy link
Member Author

sgdc3 commented Dec 29, 2015

#90

@Xephi Xephi changed the title Clean the configuration management Clean and Rework the entire Configuration Dec 31, 2015
@Xephi
Copy link
Contributor

Xephi commented Dec 31, 2015

As @sgdc3 mention me, we need to make something equivalent to Yamler does :
https://github.com/Cube-Space/Yamler/tree/master/Yamler-Core
https://www.spigotmc.org/resources/yamler.315/

By the way we actually need some things :

  • Enum System
  • Restructure and Split configuration
  • Comments support
  • Update from the old Settings to the new one

Feel free to add some tasks/comments here

@sgdc3 : I was feeling to do not split the configuration in differents .yml file, but get a splitted structure in the same config.yml , like Essentials does

@Xephi Xephi changed the title Clean and Rework the entire Configuration [DEV] Clean and Rework the entire Configuration Dec 31, 2015
@ljacqu
Copy link
Member

ljacqu commented Dec 31, 2015

enum system will be done by me

@Xephi
Copy link
Contributor

Xephi commented Dec 31, 2015

ok nice ;)

@Xephi Xephi self-assigned this Dec 31, 2015
@Xephi
Copy link
Contributor

Xephi commented Dec 31, 2015

I'm working on a new CustomConfiguration management

@ljacqu
Copy link
Member

ljacqu commented Jan 1, 2016

I have done an enum-like setup on the branch. Enums weren't possible because of generics but basically a config file would have public static final fields of Property<T>:

public static final Property<Boolean> REMOVE_PASSWORD_FROM_CONSOLE =
   // property type, property path (in the file), default value
    newProperty(PropertyType.BOOLEAN, "security.removePasswordFromConsole", true);

Retrieving a setting would be something along the lines of

String smtpServer = settings.getProperty(EmailConfig.SMTP_SERVER);

What's nice about this:

  • it's typed (compiler knows that getProperty() will return a String or whatever)
  • settings are retrieved through a method instead of accessing fields (unit testing friendly)
  • values are loaded centrally (in a map), so operations like "add missing properties and save the file if this is the case" can be done generically
  • no use of reflection

What's not nice about this:

  • when a new property is generated I put the generated property into a map so we know all properties we need to look for :( fixed.
  • more verbose than enums or public fields
  • kind of error-prone because static or final might get forgotten. A test can check for that, though.
  • something like Property<T> obviously doesn't play well if we want to define a property for List<String>, though this can be fixed

@ljacqu
Copy link
Member

ljacqu commented Jan 7, 2016

Todo @ljacqu:

  • ✓ Write proper tests for the new classes; extract EnumProperty to its own class
  • ✓ Write consistency tests (ensure public final static fields on ConfigurationClass children)
  • Verify whether we can use an existing Yaml writer (snakeyaml as used by Bukkit)
  • ✓ Add a contains() method to PropertyType
  • Add checks to see if we need to rewrite the config.yml (as currently done in Settings)
  • ✓ Add new settings to the command service
  • ✓ Migrate a handful of setting reads to use NewSettings

Since setting values are used extensively, we will need to migrate step by step to the new class, i.e. we'll have Settings and NewSettings for some time

@ljacqu
Copy link
Member

ljacqu commented Jan 17, 2016

I think it's been proven that this issue contains way too much content. Referencing this issue number, I've implemented a new settings management class based on Xephi's. Currently, it co-exists with the old settings class.
I will create follow-up issues for all other pending things so we can keep track of the goals on a more granular level. I will rename & close this issue.

@ljacqu
Copy link
Member

ljacqu commented Jan 17, 2016

#448 Restructure settings
#449 Change to new settings and delete the old one
#450 Port rewrite checks ("migrations") to the new settings class

@ljacqu ljacqu closed this as completed Jan 17, 2016
@ljacqu ljacqu changed the title [DEV] Clean and Rework the entire Configuration [DEV] Create new configuration management Jan 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants