Code review

[lenak25]

Due to the lack of a better way for reviewing a repo, we've decided to open a new issue..

Great job, very clean and compact code!

Some minor comments:

1. Docs:
   Great docs, clear and informative!

• I think you can remove the truffle installation part after your changes

2. Error checking:

• If flags are not specified in the monitor CLI, than their default values are used and the code will raise an exception (reading from a null file / initializing WebsocketProvider with null, etc)
• If there is an error in the file path/it is not a legit JSON, the code will raise an exception..
• Could you please elaborate on the No need to try/catch. Let it throw. comments?

3. Logs:

• Why monitor's logs are written to strerr? I would expect them to be written to a file or to stdout but differently (with a different prefix perhaps)

4. Peer discovery:

• No need to dial on every peer:discovery, we need to dial to one bootstrap at the beginning and that's that, the DHT will take care of connecting us to the other peers.

5. Topics:

• Please consider moving the general topic names to a constants file.

Thanks!

[assafmo]

1. 👍
2. If wrong values/no values are given as flags I thought it is okay to just let it crash with the error that caused the crash.
3. It's a common practice in Linux to write data to stdout and logs to stderr. The user can easily redirect those to wherever.
4. 👍. I think I saw this practice in libp2p's examples.
5. 👍

[assafmo]

Will fix all when I get back.

[assafmo]

4. https://github.com/libp2p/js-libp2p/tree/master/examples/discovery-mechanisms#1-bootstrap-list-of-peers-when-booting-a-node

[lenak25]

3. Makes sense, I've learned something new, thanks :)
4. I am familiar with the example, I've taken a different approach in the node implementation (dialing only when required).. Lets spent some time to understand which approach is better (not urgent)

I have only now went through the test, also looks great, several comments:

a. The check of the subscription should test the subscription itself (messages sent to a subscribed topic are received) instead of checking the monitor's logs. You are doing this for the broadcast topic, you can do the same for the taskresult and the worker topics (by using the CLI publish command).

c. I think that both use cases - testing the monitor before and after the bootstrap start - can be more compact: the connection to the bootstrap and the subscription check can be done in the same it. If you prefer it this way, please reuse code by calling to a common function.

Thanks!

[assafmo]

2. Done: bdd9c0c, d70c328, e7c944b

[assafmo]

1. Done: 98b409e

[assafmo]

3. Resolved: Code review #5 (comment)

[assafmo]

4. Done: 3d4caac

[assafmo]

5. Done: 986859f

[assafmo]

Tests:
a. I think it's best to black-box test this. This is why I'm reading the logs.

[assafmo]

Tests:
c. Done: 2c96001