MORE docs + tests #28
Conversation
|
|
||
| // *** LEGACY TIDYUP *** |
There was a problem hiding this comment.
Ummm .. I did this, which might be bad (naughty mix). To my eye, if we have config.connections we shouldn't really have suspicious config.port wandering around waiting to be accidentally used when there's a different config.connections.incoming.net[0].port been set. RIGHT?
This should really be a separate PR, but life is messy you know?
There was a problem hiding this comment.
I dig this. I don't have a solid grasp on which modules use config.port and config.host, but this would require a major version bump and some release notes, yeah?
There was a problem hiding this comment.
config.port and config.host are useful configurations because you can set them with environment variables ssb_port and ssb_host. removing these means we can't configure pubs in the standard way. that being said, my plan for PeachCloud is to avoid using these parts of the jsbot stack altogether, but recent changes are currently breaking ahdinosaur/ssb-pub: ahdinosaur/ssb-pub#15 (comment)
There was a problem hiding this comment.
oops, my above comment is probably in the wrong place, deleting config.host from the returned object is good, not being able to set ssb_host to configure the external host is meh.
There was a problem hiding this comment.
Cool, so that sounds like a 👍 from @ahdinosaur which is good enough for me.
What I've got supports old inputs, but also removes a footgun!
There was a problem hiding this comment.
turns out this was needed by ssb-server/bin. It's not documented anywhere, sure. and even I had forgotten that was used. But there generally tend to be things like this. The best way to catch things like this is to symlink a module into it's dependants and then run those tests.
cd ssb-server; cd node_modules; rm -rf ssb-keys; ln -s ../../ssb-keys; cd ..; npm test
This is the last line of defence against releasing broken stuff, and it would make me very happy if people making PRs would run this before they submit it for review.
|
|
||
| // *** LEGACY TIDYUP *** |
There was a problem hiding this comment.
I dig this. I don't have a solid grasp on which modules use config.port and config.host, but this would require a major version bump and some release notes, yeah?
| - `scope` *(string)* ... TODO | ||
| - `private` - TODO | ||
| - `public` - TODO | ||
| - `external` *(array of strings)* ... TODO |
There was a problem hiding this comment.
Thanks for leaving TODOs, this makes it much easier to figure out what needs to be documented in the future.
There was a problem hiding this comment.
tunnel: is also a type of transport that may occur, see https://github.com/ssbc/ssb-tunnel . If it's still a valid plugin, I kind of want to see it used more.
I have also seen 'device' and 'local' used in scopes.
There was a problem hiding this comment.
thanks for these additions @hamnox , I'll add them in as stubs!
README.md
Outdated
| @@ -29,54 +61,95 @@ There are some configuration options for the sysadmins out there. All configurat | |||
| * `path` *(string)* Path to the application data folder, which contains the private key, message attachment data (blobs) and the leveldb backend. Defaults to `$HOME/.ssb`. | |||
| * `master` *(array)* Pubkeys of users who, if they connect to the Scuttlebot instance, are allowed to command the primary user with full rights. Useful for remotely operating a pub. Defaults to `[]`. | |||
| * `logging.level` *(string)* How verbose should the logging be. Possible values are error, warning, notice, and info. Defaults to `notice`. | |||
| * `remote` ... TODO (this was formerly undocumented but came up in conversation) | |||
There was a problem hiding this comment.
this remote thing (naively) looks like redundency. If it's really needed should it not be derived from what's in config.connections?
There was a problem hiding this comment.
That would make sense, are you thinking something like an index number on the config.connections array?
There was a problem hiding this comment.
Please don’t touch Remote just yet. It is used by nearly all clients. Most known for remote = ws://... by patchbay and liteclients.
I share the sentiment that in some future connections should handle this somehow but just removing it without solid tests will give a lot of people sleepless nights.
There was a problem hiding this comment.
Good to know! I'll leave a note in the README about this for future butts
|
|
||
| ### `connections` | ||
| Deprecated Options: |
There was a problem hiding this comment.
Great idea! Probably a good idea to warn about that if they are overwritten.
There was a problem hiding this comment.
do you mean that the raw values are being removed from the final config? I've added a note about that now
There was a problem hiding this comment.
Given what dinosaur said about env variables scratch my comment.
|
So I'm super excited to get this merged. Shall I merge it as-is (with all the TODO stubs)? I'd be super excited if the people who wrote or know what those fields unknown to me would fill them in, even with just a little detail... pretty much anything is better than an empty TODO right? |
|
the documentation and tests PR is a very rare breed and highly prized by collectors. thank you @mixmix for this wonderful specimen! |
|
merged into 2.3.8 |
|
thanks @dominictarr - yeah I guess you missed this #28 (comment) where I declared I might be doing something naughty. I'll chase the problem down and see if I can fix it |
|
Cool, I see you fixed it in 2.3.9 I'm gonna see if I can restore that code and fix ssb-server |
|
ech ... |
I'm trying to learn how to configure
ssb-server. I was pointed here to learn aboutconnections. I found the docs confusing, and I also noticed the tests were pretty lacking.This is a stab at an improvement:
1. added tests
🚀 In the future I want to come back and write a JSON-schema for this config bizo - this currently just feels like it's waiting to catch on fire because there are so many place you can stick something wrong
2. clearer documentation
I got confused reading these docs. And I was confused last time I looked. Finding out what's valid config is hard because this config plugs into a big turtle-stack, and I have no idea what's consuming what from here.
ssb-serverTODOs in empty spots