Improved CONTRIBUTING.md file #117

jirihnidek · 2023-05-05T15:02:00Z

Fixed one issue with example
Added instruction how to start mosquitto server
Replaced test.mosquito.org with localhost, because it is really easy to run mosquitto server
Wrapped too long lines
Added some notes

subpop

Thank you for the suggestions. I have a few ideas on how to keep the document organized as well.

subpop · 2023-05-05T15:21:36Z

CONTRIBUTING.md

 ```

 You should see the message logged to the output of `sub` in [Terminal
 3](#terminal-3) and receipt of the message logged in the output of `yggd` in
-[Terminal 1](#terminal-1).
+[Terminal 1](#terminal-1). The `echo` worker does not receive any message in this


I don't think it makes sense to declare what won't happen. If that were necessary, we would have to document what won't happen a lot, in many places. My guess is that you thought the ping would send a message to the echo worker; so perhaps we should reorder these sections to send the ping command before starting the echo worker. This shows that MQTT is working, without confusing things by starting the echo worker first.

I tried the reordering of terminals and it would require switching of terminals. I will add more subsections to section Terminal 4 to emphasize that we send only Ping command and we test sending MQTT command to yggd.

subpop · 2023-05-05T15:24:33Z

CONTRIBUTING.md

+`mosquitto` is extremely easy to set up. When you do not do any configuration,
+and you start the service like this:
+
+```bash
+systemctl start mosquitto
+```
+
+The broker will listen on port 1883 for unencrypted connections.


Let's move this to the # Quickstart section, perhaps as a "before starting your terminals" section.

Suggested change

`mosquitto` is extremely easy to set up. When you do not do any configuration,

and you start the service like this:

```bash

systemctl start mosquitto

```

The broker will listen on port 1883 for unencrypted connections.

`mosquitto` is extremely easy to set up. The broker will listen on port 1883 for unencrypted

connections without any configration changes.

And add a reference to starting the service before starting up the terminals.

subpop · 2023-05-05T15:25:08Z

CONTRIBUTING.md


-Now publish a data message to the echo worker.
+Now publish a data message "hello" to the `echo` worker.


Suggested change

Now publish a data message "hello" to the `echo` worker.

Now publish a data message containing the string "hello" to the `echo` worker.

subpop · 2023-05-05T15:26:14Z

CONTRIBUTING.md

+> Note: if you implemented your own worker (e.g. called cool_worker),
+> you will need to replace the `echo` directive with the name of your
+> worker (--directive cool_worker).
+


These doesn't need to be part of the quickstart. This knowledge will come later when the reader begins to explore what a worker is and how to communicate with them. Not all readers will be worker developers. There is also a Worker Developer Guide on the wiki.

subpop · 2023-05-05T15:28:02Z

CONTRIBUTING.md

+There is another option how to test the echo worker. You can use the `yggctl`
+like this:
+
+```bash
+echo -n "hello" | go run ./cmd/yggctl dispatch -w "echo" -
+```
+


I'd like to keep the quickstart as short and simple as possible. If we don't have a section below on yggctl's capabilities, then let's add one. Otherwise, can we move this to that section?

OK, I will move it to section related to yggctl.

subpop · 2023-05-05T15:29:32Z

CONTRIBUTING.md

+sub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/data/in \
+    -topic yggdrasil/$CLIENT_ID/data/out -topic yggdrasil/$CLIENT_ID/control/out


Since we're using $(hostname) for client ID in the quickstart, let's change the references here too.

Suggested change

sub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/data/in \

-topic yggdrasil/$CLIENT_ID/data/out -topic yggdrasil/$CLIENT_ID/control/out

sub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/data/in \

-topic yggdrasil/$(hostname)/data/out -topic yggdrasil/$(hostname)/control/out

subpop · 2023-05-05T15:30:01Z

CONTRIBUTING.md

@@ -211,7 +234,8 @@ A client can be sent a `PING` command by generated a control message and
 publishing it to the client's "control/in" topic:

 ```
-yggctl generate control-message --type command '{"command":"ping"}' | pub -broker tcp://test.mosquitto.org:8883 -topic yggdrasil/$CLIENT_ID/control/in
+yggctl generate control-message --type command '{"command":"ping"}' | \
+    pub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/control/in


Suggested change

pub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/control/in

pub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/control/in

subpop · 2023-05-05T15:30:10Z

CONTRIBUTING.md

@@ -222,7 +246,8 @@ Similarly, a data message can be published to a client using `yggctl generate`
 and `pub`.

 ```
-yggctl generate data-message --directive echo hello | pub -broker tcp://test.mosquitto.org:8883 -topic yggdrasil/$CLIENT_ID/data/in
+yggctl generate data-message --directive echo hello | \
+    pub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/data/in


Suggested change

pub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/data/in

pub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/data/in

subpop

I like the approach in the "Terminal 4" section. It more explicitly indicates "Terminal 4" is the interactive terminal; the place where you type stuff to make stuff happen in the other terminals.

subpop · 2023-05-09T13:50:06Z

CONTRIBUTING.md

+### Mosquitto
+
+When you do not make any changes of configuration, and you start the service like this:
+
+```bash
+systemctl start mosquitto
+```
+
+The broker will listen on port 1883 for unencrypted connections (`tcp://localhost:1883`).
+We will use this fact in following examples.
+


Suggested change

### Mosquitto

When you do not make any changes of configuration, and you start the service like this:

```bash

systemctl start mosquitto

```

The broker will listen on port 1883 for unencrypted connections (`tcp://localhost:1883`).

We will use this fact in following examples.

This quickstart assumes an unencrypted MQTT broker is listening on `localhost:1883`.

Typically, all that is required to start the `mosquitto` broker is: `systemctl start mosquitto`.

I prefer as much brevity as possible in a quickstart guide.

subpop · 2023-05-09T13:50:55Z

CONTRIBUTING.md

+#### Testing MQTT
+
+Publish a "PING" command to the `yggd` "control/in" topic to test sending
+MQTT command to `yggd`.


Suggested change

MQTT command to `yggd`.

an MQTT message to `yggd`.

subpop · 2023-05-09T13:53:06Z

CONTRIBUTING.md

@@ -202,7 +221,8 @@ message to the broker, destined for one of the topics `yggd` is subscribed to.
 To watch a topic for messages, subscribe to it with `sub`:

 ```
-sub -broker tcp://test.mosquitto.org:8883 -topic yggdrasil/$CLIENT_ID/data/in -topic yggdrasil/$CLIENT_ID/data/out -topic yggdrasil/$CLIENT_ID/control/out
+sub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/data/in \
+    -topic yggdrasil/$(hostname)/data/out -topic yggdrasil/$(hostname)/control/out
 ```

 ## Publish a message


Suggested change

## Publish a message

## Publish a message

### Over MQTT

subpop · 2023-05-09T13:55:53Z

CONTRIBUTING.md

+There is another option how to test the `echo` worker. You can use the `yggctl`
+like this and send message containing "hello" to the `echo` worker over D-Bus:
+
+```bash
+echo -n "hello" | go run ./cmd/yggctl dispatch -w "echo" -


Suggested change

There is another option how to test the `echo` worker. You can use the `yggctl`

like this and send message containing "hello" to the `echo` worker over D-Bus:

```bash

echo -n "hello" | go run ./cmd/yggctl dispatch -w "echo" -

### Directly

It is also possible to send data directly to a worker. This method does not publish

a message to the MQTT broker. Instead, it calls a D-Bus method:

```bash

echo -n "hello" | yggctl dispatch -w "echo" -

None of the other examples in this section use go run ./cmd/yggctl. We should be consistent.

* Fixed one issue with example * Added instruction how to start mosquitto server * Replaced test.mosquito.org with localhost, because it is really easy to run mosquitto server * Replaced remaining $CONTROL_id with $(hostname) * Wrapped too long lines * Extended and changed some texts little bit

jirihnidek force-pushed the jhnidek/fix_contributing.md branch from 23a5ad3 to 2cfaf0d Compare May 5, 2023 15:03

subpop requested changes May 5, 2023

View reviewed changes

jirihnidek force-pushed the jhnidek/fix_contributing.md branch from 2cfaf0d to 7d52ad0 Compare May 9, 2023 09:41

subpop requested changes May 9, 2023

View reviewed changes

jirihnidek force-pushed the jhnidek/fix_contributing.md branch from 7d52ad0 to 07be00f Compare May 10, 2023 08:56

subpop approved these changes May 10, 2023

View reviewed changes

subpop merged commit 7797bf3 into RedHatInsights:main May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved CONTRIBUTING.md file #117

Improved CONTRIBUTING.md file #117

jirihnidek commented May 5, 2023

subpop left a comment

subpop May 5, 2023

jirihnidek May 9, 2023

subpop May 5, 2023

jirihnidek May 9, 2023

subpop May 5, 2023

subpop May 5, 2023

subpop May 5, 2023

jirihnidek May 9, 2023 •

edited

Loading

subpop May 5, 2023

subpop May 5, 2023

subpop May 5, 2023

subpop left a comment

subpop May 9, 2023

jirihnidek May 10, 2023

subpop May 9, 2023

jirihnidek May 10, 2023

subpop May 9, 2023

jirihnidek May 10, 2023

subpop May 9, 2023

jirihnidek May 10, 2023


		Now publish a data message to the echo worker.
		Now publish a data message "hello" to the `echo` worker.

	Now publish a data message "hello" to the `echo` worker.
	Now publish a data message containing the string "hello" to the `echo` worker.

		sub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/data/in \
		-topic yggdrasil/$CLIENT_ID/data/out -topic yggdrasil/$CLIENT_ID/control/out

	pub -broker tcp://localhost:1883 -topic yggdrasil/$CLIENT_ID/control/in
	pub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/control/in

-There is another option how to test the `echo` worker. You can use the `yggctl`
-like this and send message containing "hello" to the `echo` worker over D-Bus:
-```bash
-echo -n "hello" | go run ./cmd/yggctl dispatch -w "echo" -
+### Directly
+It is also possible to send data directly to a worker. This method does not publish
+a message to the MQTT broker. Instead, it calls a D-Bus method:
+```bash
+echo -n "hello" | yggctl dispatch -w "echo" -

Improved CONTRIBUTING.md file #117

Improved CONTRIBUTING.md file #117

Conversation

jirihnidek commented May 5, 2023

subpop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirihnidek May 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subpop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirihnidek May 9, 2023 •

edited

Loading