Smokedetector: Migrate config to globalvariables

Created on 7 Jul 2016  Â·  10Comments  Â·  Source: Charcoal-SE/SmokeDetector

There are currently only a few values in config. I propose we remove "config" completely and migrate these values to globalvaribles.

This removes two locations (config and globalvariables) that developers have to modify when starting SmokeDetector and allows us to remove a ConfigParser import that is only used at start up.

feature request

Most helpful comment

We should let as many things be empty as possible. The fewer things you have to set before running the bot, the easier it is to get started.

All 10 comments

Having to edit global vars instead of a single config file would mean that an update to any place in global vars would break automatic pulling using !!/pull because git cannot merge files that have uncommitted changes.

I suggest we migrate global vars to config instead.

There are quiet a few that are important to change when running a few
version.

One of those is the rooms and privileged users. That is a dictionary. The
configuration files are strings.

However, I do agree that we don't want to pull individual user changes to
globalvaribles
On Jul 7, 2016 3:00 PM, "Fernando van Loenhout" [email protected]
wrote:

Having to edit global vars instead of a single config file would mean that
an update to any place in global vars would break automatic pulling using
!!/pull because git cannot merge files that have uncommitted changes.

I suggest we migrate global vars to config instead.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231190064,
or mute the thread
https://github.com/notifications/unsubscribe/AGKb5wIXiJoDympr3u1Fn1zkgcsZOvbUks5qTVrUgaJpZM4JHcM5
.

Another thing to consider is that the MS key in that is fairly sensitive,
and it's nice having a hard barrier (.gitignore) to me accidentally
committing/pushing credentials.

On Thu, Jul 7, 2016, 2:03 PM A Wegner [email protected] wrote:

There are quiet a few that are important to change when running a few
version.

One of those is the rooms and privileged users. That is a dictionary. The
configuration files are strings.

However, I do agree that we don't want to pull individual user changes to
globalvaribles
On Jul 7, 2016 3:00 PM, "Fernando van Loenhout" [email protected]
wrote:

Having to edit global vars instead of a single config file would mean
that
an update to any place in global vars would break automatic pulling using
!!/pull because git cannot merge files that have uncommitted changes.

I suggest we migrate global vars to config instead.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231190064
,
or mute the thread
<
https://github.com/notifications/unsubscribe/AGKb5wIXiJoDympr3u1Fn1zkgcsZOvbUks5qTVrUgaJpZM4JHcM5

.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231190761,
or mute the thread
https://github.com/notifications/unsubscribe/AE7FZms6CW58ZjOHlCblqHp3YX2tyeudks5qTVtygaJpZM4JHcM5
.

I don't see config in .gitignore currently. We may want to fix that too. Or put the key in an environment variable the way we do with login credentials.

The gitignore change is probably stuck in my local repo. An environment variable could work.

An environment variable could work.

Nay, an environment variable would not only work, it would be a good idea in terms of security.

It seems we've decided what to do with the MS Key - put it in an environment variable.

What is the opinion on the other values in config?

I don't see any reason that they shouldn't be environment variables either.
It should probably allow start without metasmoke information, but perhaps
require the location (Running on [location]) to be present, either as an
environment variable or as a command line option.

Your other suggestion of being able to change the [SmokeDetector] prefix
and repo link would also fit in well - perhaps try to read from environment
variables / command line options, then fall back on defaults?

On Fri, Jul 8, 2016, 11:52 AM A Wegner [email protected] wrote:

It seems we've decided what to do with the MS Key - put it in an
environment variable.

What is the opinion on the other values in config?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231427317,
or mute the thread
https://github.com/notifications/unsubscribe/AE7FZuBhvPUDazNEE3-gz2IzEXAZL8S_ks5qTo5pgaJpZM4JHcM5
.

I am happy with environment variables and default fullbacks for both
location and the chat prefix strings.

The MSkey should allow for nonexistence (since I don't run MS locally when
testing smokey).
On Jul 8, 2016 12:57 PM, "Undo1" [email protected] wrote:

I don't see any reason that they shouldn't be environment variables either.
It should probably allow start without metasmoke information, but perhaps
require the location (Running on [location]) to be present, either as an
environment variable or as a command line option.

Your other suggestion of being able to change the [SmokeDetector] prefix
and repo link would also fit in well - perhaps try to read from environment
variables / command line options, then fall back on defaults?

On Fri, Jul 8, 2016, 11:52 AM A Wegner [email protected] wrote:

It seems we've decided what to do with the MS Key - put it in an
environment variable.

What is the opinion on the other values in config?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
<
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231427317
,
or mute the thread
<
https://github.com/notifications/unsubscribe/AE7FZuBhvPUDazNEE3-gz2IzEXAZL8S_ks5qTo5pgaJpZM4JHcM5

.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Charcoal-SE/SmokeDetector/issues/170#issuecomment-231428653,
or mute the thread
https://github.com/notifications/unsubscribe/AGKb5wtt31H4Xy8nkzdRzlDbC8EqApc2ks5qTo-SgaJpZM4JHcM5
.

We should let as many things be empty as possible. The fewer things you have to set before running the bot, the easier it is to get started.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Cerbrus picture Cerbrus  Â·  8Comments

Tyler-H picture Tyler-H  Â·  3Comments

tripleee picture tripleee  Â·  4Comments

fortunate-man picture fortunate-man  Â·  3Comments

angussidney picture angussidney  Â·  9Comments