w3c / scribejs

Converter of RRSAgent IRC logs into minutes in markdown
https://w3c.github.io/scribejs/BrowserView/
Other
11 stars 14 forks source link

Improve configuration management #22

Closed tripu closed 7 years ago

tripu commented 7 years ago

Following common practices:

Fixes #10.

iherman commented 7 years ago

@tripu I am not sure I agree with this line of thought.

The same script can be used to generate minutes for different WG-s. Each of them require their own configuration file. However, the user also has some data that are common to all of them but, maybe more importantly, should not be shared with anyone else: the github credentials are the typical cases. Hence the need, in my view, for the $HOME/.scribejs.json file. It is not necessary to have it, but it is a useful feature for a user that manages several groups (like me:-)

iherman commented 7 years ago

Bottomline: I would prefer to close this without merge.

tripu commented 7 years ago

@iherman, I'm not sure I understand your comments, sorry.

“The same script can be used to generate minutes for different WG-s. Each of them require their own configuration file.”

If the same computer or user generates minutes for more than one group, scribejs will always have to be invoked passing along the name of the specific config file for each group, $ scribejs -c <FILE>, right? And in that case, it doesn't matter where all those config files live (the user's home dir, the current dir, or somewhere else).

“However, the user also has some data that are common to all of them but, maybe more importantly, should not be shared with anyone else: the github credentials are the typical cases.”

How is that managed now? Aren't those GH credentials redundant, present in each and every config file?

The “not be shared with anyone else” requirement is orthogonal to this PR, isn't it? If it's a workstation, config files can be anywhere and they'll be safe. If it's one of our servers, W3C administrators will have access to them anyway (and nobody else).

iherman commented 7 years ago

@tripu,

at present, there are three ways paramters can be set; these are merged. These are (in decreasing priority)

  1. Command line arguments
  2. A config file identified via the -c argument
  3. $HOME/.scribe.json

The third, user level config file contain those that are common for all working groups. The typical case is to put GH credentials there, which otherwise would have to be repeated over all configurationn files.

Note that doing it this way is also a security issue. I may share the working group level config file with others who want to use the tools, but I do not want to share my credentials. If we remove the third option above, this cannot be done...

iherman commented 7 years ago

@tripu, It seems that the latest change of .gitignore has created merge conflicts again:-( Can you look at that, too?

tripu commented 7 years ago

@iherman:

“at present, there are three ways paramters can be set; these are merged. These are (in decreasing priority): 1. Command line arguments. 2. A config file identified via the -c argument. 3. $HOME/.scribe.json. The third, user level config file contain those that are common for all working groups. The typical case is to put GH credentials there, which otherwise would have to be repeated over all configurationn files.”

Ah, I remember now that you had this clever “cascading” for settings… Okay.

“Note that doing it this way is also a security issue. I may share the working group level config file with others who want to use the tools, but I do not want to share my credentials. If we remove the third option above, this cannot be done...”

But this PR does not remove that third option — it just changes the default filename, from ~/.scribejs.json (≡ $HOME/.scribejs.json) to ./config.json. That keeps the user's home directory clean of config files. All other considerations about keeping that file safe, sharing it or not, etc. remain unchanged.

“It seems that the latest change of .gitignore has created merge conflicts again:-( Can you look at that, too?&rdqo;

Rebased. Note that while resolving conflicts I changed your two lines in .gitignore to be more generic.

(Better not to commit to master directly if it can be avoided. I can review your PRs, if you want :)

iherman commented 7 years ago

(Point well taken on PRs, sorry!)

What is wrong with having these config file in the user's top directory? It is a fixed place, easy to maintain. ./config.json will depend where you are in when you start the script, that is fairly error prone! E.g., I may want to run the script on my local partial clone of the W3C web site, i.e., I would run it in, say, /2017/08 and, in a week, in /2017/09; indeed, that may be where the local copy of the IRC logs are (that I get via CVS). In your model I would have to copy (or symlink) ./config.js in both local directories, in my model there is nothing to do.

There is something I do not get...

iherman commented 7 years ago

@tripu I saw that you did some changes and, in some sense, you backtracked some of the changes. Does it mean that you consider that as 'mergable' ?

tripu commented 7 years ago

@iherman, I think it's just a matter of different approaches to configuration management; I can't claim mine is objectively better, and I think yours has disadvantages, too.

“What is wrong with having these config file in the user's top directory? It is a fixed place, easy to maintain.”

It goes against the general rule of not polluting the user's home directory.

./config.json will depend where you are in when you start the script, that is fairly error prone!”

Opposite case: user plays a bit with scribejs, then removes it, and forgets about it. Some time later, they install it again (and assume they start with a fresh installation). But, for the love of $DEITY, they can't fathom why the software is misbehaving, as if it were configured with some parameters out-of-the box… until they realise that even after they uninstalled it the last time, a fastidious hidden file stayed in their home directory, and that file is now being used. When software is self-contained, it's easier to have a clear picture of where its stuff is.

Anyway, I've updated this branch, and the description of the PR, to revert my proposal: back to ~/.scribejs.json. The rest of my changes are still here.

Can you review again, please?

iherman commented 7 years ago

Hm.

It goes against the general rule of not polluting the user's home directory.

Isn't this just wishful thinking? Looking at my home directory, I have, just installed lately

and, of course all the .bashrc, .vimrc etc files of this world. Yest, it is full of it, yes, for more complex applications Mac tries to get people to pollute Library instead, etc, but that boat has sailed more than 30 years ago! :-)

I will merge the PR, I have already looked at it...

tripu commented 7 years ago

Thanks, @iherman!

“Isn't this just wishful thinking? […] that boat has sailed more than 30 years ago! :-)”

Since when is that a valid reason to not do The Right Thing? :)

node_modules, used by node.js (note that it is not even .node_modules which would be better)”

That must be because at least once you have run npm i <package> in your home dir. npm creates that subdirectory every time you ask it to install some package (unless you ask it to install globally, with -g). Unless you are working on some software in your home dir, I bet you can remove that directory.