lloeki / matterfront

Mattermost frontend app for OS X, Windows and Linux
MIT License
152 stars 27 forks source link

Switch config to nconf #47

Closed geekytime closed 8 years ago

geekytime commented 8 years ago

This moves the app config into a settings module that is built around nconf. This will allow us to have a nicely layered configuration system, including default values, command-line args, and multiple config files. We could also support environment variables if necessary.

I've moved the team urls into the app state and put it in an array, as a step toward supporting having multiple teams open at the same time, which is my main goal right now.

LongLiveCHIEF commented 8 years ago

Can you update the README with the new usage/build instructions this would entail?

geekytime commented 8 years ago

Can you update the README with the new usage/build instructions this would entail?

Yup. Sorry.

LongLiveCHIEF commented 8 years ago

The state.json object needs to be configured so that you can set up a server, without specifying teams (teams are optional, servers are not). the conf should focus on state.host instead of state.teams. I think the object that results by the time it resolves from nconf should look like this (the server/teams piece anyways):

{
  "hosts": [{
    "url": "http://some.mattermost.com/",
    "teams":[{
        "name":"team1",
        "path":"/team-1" 
    },{
        "name":"team2",
        "path":"/team-2"
    }] 
  }],
  "window": {
    "width": 800
  }
}

Each host object would then also be able to take other configuration parameters we need to be able to add down the road, such as proxy, cert, certPath, and an auth object for configuration of auth protocol params (such as Oath2 endpoints and keys/secrets, or LDAP server bind info)

geekytime commented 8 years ago

Each host object would then also be able to take other configuration parameters we need to be able to add down the road, such as proxy, cert, certPath, and an auth object for configuration of auth protocol params (such as Oath2 endpoints and keys/secrets, or LDAP server bind info)

Why should we make this more complicated than it needs to be? Since Mattermost currently thinks in teams, I think we should probably wait and do this once Mattermost and Matterfront support any of this stuff.

This is especially true for the auth stuff. I don't think we want Matterfront to EVER store credentials or secrets or anything, since Mattermost will want to do that, and then use cookies or tokens for auth.

geekytime commented 8 years ago

By storing the teams in the state, we make it simple for the user to simply copy/paste the url for their team right into Matterfront.

I'd rather wait and see what kinds of config is actually required by us and our other users moving forward, and expand this at that point based on our actual needs.

geekytime commented 8 years ago

(teams are optional, servers are not)

But humans don't think in terms of servers. They think in terms of teams. There are some things that Matterfront just doesn't need to be able to do, and I think connecting to the general server and signing up for new teams could be one of those, at least at first.

LongLiveCHIEF commented 8 years ago

You're thinking humans... a.k.a individual one off users. I'm thinking IT admins, responsible for the configuration and build of the executable (which is where the configuration lies), which they distribute to their organization... which has n number of teams.

Things like the host settings I described are also organization level settings.

Mattermost does not actually think in teams, it thinks in organizations. The average everyday user is just unable to see the organization level features. Matterfront needs those features.

Mattermost also has the concept of "team directories", and has features designed to allow an onboarding process that depends on the user being able to visit the "home" page. These are features that don't exist in Slack.

The nature of the Mattermost "home screen" will continue to evolve and play a more important role for organizations, and it's a simple matter for us to change the config structure now to easily accommodate this now and into the future. (so that we can allow either users OR admins to decide which experience they want to deliver with the compiled executable).

LongLiveCHIEF commented 8 years ago

I'd rather wait and see what kinds of config is actually required by us and our other users moving forward, and expand this at that point based on our actual needs.

We've already had these discussions though. As a new contributor, you might see it as a wait and see... while for us, we've already been through that phase, and are just trying to explain the outcome and what we need to support.

geekytime commented 8 years ago

As a new contributor, you might see it as a wait and see...

It's not a "wait and see whether we need it", it's a "wait and see the best way to implement it". I tend to apply the KISS principle first, and only make things complex when they need to get complex. Small, iterative changes that support the current needs.

You're thinking ahead to features that don't exist, yet: http://www.danielsen.com/jokes/objecttoaster.txt

LongLiveCHIEF commented 8 years ago

Let me take a quick nap and consider your arguments. Check back in about 90 minutes.

Maybe @lloeki will have had a chance to share any thoughts on this implementation by then. On Dec 3, 2015 10:46 AM, "Chris Jaynes" notifications@github.com wrote:

As a new contributor, you might see it as a wait and see...

It's not a "wait and see whether we need it", it's a "wait and see the best way to implement it". I tend to apply the KISS principle first, and only make things complex when they need to get complex. Small, iterative changes that support the current needs.

You're thinking ahead to features that don't exist, yet: http://www.danielsen.com/jokes/objecttoaster.txt

— Reply to this email directly or view it on GitHub https://github.com/lloeki/matterfront/pull/47#issuecomment-161711399.

LongLiveCHIEF commented 8 years ago

ok, so thought about this. You are right that it should be teams. However, There will need to be a host at the same level as window, and those org-level properties such as proxy/auth/etc... will also be at that level.

That's something that we can do in the future though.

lloeki commented 8 years ago

Sorry for not chiming in :-/ very busy ATM.

LongLiveCHIEF commented 8 years ago

no worries. Just let us know if you have issues with any of our PR's. I have a good feel for what you would want to see in code reviews, so I'm trying to make sure I cover any concerns you would have brought up if you haven't been able to comment.