CivClassic / AnsibleSetup

Civclassic 1.16.5 Ansible deployment environment
5 stars 39 forks source link

Separate hack configs into their own files #235

Closed Protonull closed 2 years ago

Protonull commented 3 years ago

These are the required config changes, assuming https://github.com/CivClassic/SimpleAdminHacks/pull/87 is merged.

wingzero54 commented 3 years ago

I can't say I'm a fan of this. It is a problem, but I'm not sure this is the solution we should be pursuing

Convoy20 commented 3 years ago

I can't say I'm a fan of this. It is a problem, but I'm not sure this is the solution we should be pursuing

Not a fan of the modulization, or not a fan of splitting up the configs into a bunch of different parts?

wingzero54 commented 2 years ago

I can't say I'm a fan of this. It is a problem, but I'm not sure this is the solution we should be pursuing

Not a fan of the modulization, or not a fan of splitting up the configs into a bunch of different parts?

2 months too late but not a fan of splitting the configs into a bunch of different files, makes it a lot harder to work with SAH config. I don't ever remember what part things are in

Protonull commented 2 years ago

I do think we need to come up with some solution though given the amount of config formatting errors we've had. Given that SimpleAdminHacks is a staple civ-plugin in its own right these days, when it breaks, it takes quite a few vital features (like elevators) with it. DogFacts should be switched over to MiniMessage, yes but we need a better way to ensure config-safety. Splitting each hack into its own config mitigates any faults to that hack specifically, rather than taking down the entire plugin.

Maxopoly commented 2 years ago

I am also opposed to this. We should do https://github.com/CivClassic/AnsibleSetup/issues/108 and leave it at that.