Seagate / halon

High availability solution
Apache License 2.0
1 stars 0 forks source link

Write developer documentation. #1290

Closed 1468ca0b-2a64-4fb4-8e52-ea5806644b4c closed 8 months ago

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: nc6

Not yet ready to merge.

[skip ci]

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: nc6

This is specified just prior.

multiple ways to bind a PhaseM block to a handle, corresponding to the "guard" to enter that phase: It's not specific to directly - it applies to all of these points.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Please also add

It "binds" a body to a phase handler.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Please reformat long lines to fit 80 characters. Take care not to break bulleted items.

This is the patch, in case you need one:

diff --git a/doc/developer/interfaces/sspl.rst b/doc/developer/interfaces/sspl.rst
index bc03765c..3c366b91 100644
--- a/doc/developer/interfaces/sspl.rst
+++ b/doc/developer/interfaces/sspl.rst
@@ -1,26 +1,49 @@
 Interface: SSPL
 ===============

-The term SSPL covers a pair of components in the Castor system with somewhat separate purposes:
+The term SSPL covers a pair of components in the Castor system with
+somewhat separate purposes:

-- SSPL-LL is a suite of sensors and actuators which abstract over the hardware and operating system upon which Halon runs. Halon uses it both to learn information about system events, and to take action in response to failures.
-- SSPL-HL runs on the CMU and acts as the public interface to castor management. It pulls some of its information from Halon.
+- SSPL-LL is a suite of sensors and actuators which abstract over the
+  hardware and operating system upon which Halon runs. Halon uses it
+  both to learn information about system events, and to take action in
+  response to failures.
+- SSPL-HL runs on the CMU and acts as the public interface to castor
+  management. It pulls some of its information from Halon.

-There are two Halon :ref:`services<../concepts/services.rst>` which are responsible for communication with these components. We refer to these as `halon:sspl` and `halon:sspl-hl` respectively.
+There are two Halon :ref:`services<../concepts/services.rst>` which
+are responsible for communication with these components. We refer to
+these as `halon:sspl` and `halon:sspl-hl` respectively.

-The interface to both of these components is over RabbitMQ message exchange. 
+The interface to both of these components is over RabbitMQ message exchange.

 RabbitMQ connections
 --------------------

-The logic for connecting to RabbitMQ is largely abstracted in order to be shared between the `halon:sspl` and `halon:sspl-hl` services. One important thing to note is that, in order for messages to be delivered, appropriate queues, exchanges and bindings must be set up on the RabbitMQ server. Since components at either end may start first, we allow either end to establish these entities. This means that we must make sure that the declarations for these things are synhronised between components, because communication may fail where each end has a different view on channel/queue properties (e.g. persistence).
+The logic for connecting to RabbitMQ is largely abstracted in order to
+be shared between the `halon:sspl` and `halon:sspl-hl` services. One
+important thing to note is that, in order for messages to be
+delivered, appropriate queues, exchanges and bindings must be set up
+on the RabbitMQ server. Since components at either end may start
+first, we allow either end to establish these entities. This means
+that we must make sure that the declarations for these things are
+synhronised between components, because communication may fail where
+each end has a different view on channel/queue properties
+(e.g. persistence).

 Schemata
 --------

-All messages between Halon and the two separate SSPL components are JSON encoded. The type of all possible messages is controlled by json-schema documents, from which Haskell bindings are automatically generated.
+All messages between Halon and the two separate SSPL components are
+JSON encoded. The type of all possible messages is controlled by
+json-schema documents, from which Haskell bindings are automatically
+generated.

-The `sspl` package contains both the schemata (in the `app` directory) and the generated bindings (in the `src` directory). If the schemata are updated and the bindings need to be regenerated, then this can be done using the `mkBindings` executable. Options can be specified if e.g. additional instances need to be generated.
+The `sspl` package contains both the schemata (in the `app` directory)
+and the generated bindings (in the `src` directory). If the schemata
+are updated and the bindings need to be regenerated, then this can be
+done using the `mkBindings` executable. Options can be specified if
+e.g. additional instances need to be generated.

 Code Pointers
 -------------
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Move “The MM replicator” to the beginning of the second item.

- The MM replicator is responsible [...]
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

No hyphenation, please. It may be rendered incorrectly; e.g. “co- incides”.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

I'm not sure the documentation generator will handle broken :ref:-s correctly. Have you tried building the documentation?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[nit] Trailing whitespace.

Kindly fix the warnings returned by git diff --check origin/master... command.

$ git diff --check origin/master...
doc/developer/components/recovery-coordinator.rst:15: trailing whitespace.
+Rules 
doc/developer/index.rst:17: new blank line at EOF.
doc/developer/interfaces/sspl.rst:11: trailing whitespace.
+The interface to both of these components is over RabbitMQ message exchange. 
doc/developer/overview.md:175: new blank line at EOF.
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: nc6

OK, I have consulted with my documentation consultant, and she has taken your side :-) So I will limit the line width!

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

The whole purpose of documentation is to be read. And readers' convenience is more important than writer's, because the text is written once but read many times.

I had line wrapping turned on; see the screenshot above. Unfortunately, line wrapping is not particularly clever and breaks words apart in the middle. Such broken words are difficult to read (see the output of less doc/developer/components/resource-graph.rst).

[...] inserting terms into a paragraph might require re-flowing multiple lines.

Refilling a paragraph is easy: M-q in Emacs, gqap in Vim. It is also possible to enable automatic refilling.

It is normal for a software project to limit width of lines in its documentation. Examples: Linux, git, Cloud Haskell.


In any case, I do not insist that refilling is made in this patch. That's an easy thing to do and I can do it myself later. (Though if I do that, then git-annotate will show me as an author... @nc6, would you mind if I added a refilling paragraphs commit to this PR? In this case the squashed commit would be of your authorship.)

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: nc6

Turn on line wrapping. Manual wrapping of lines is just okay for code, but a terrible idea for prose, in which inserting terms into a paragraph might require re-flowing multiple lines.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: nc6

I have added these files as notes to myself on what should be documented :-)

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[optional] s/easiest/easier/ ?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Please consider making this paragraph a footnote, to emphasize that this is a nice-to-have thing and not a description of existing functionality.


There are two possible cardinalities[#]_, ...

.. [#] It might also be nice ...
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Shouldn't it be spelled as ‘Cloud Haskell’? (Or ‘C-H’.)

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Reading such long lines in terminal or not-super-wide editor window is not a pleasant experience. Please split them to make 75–80 characters wide.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

I see no point of putting two identical references (recovery-coordinator) in the same sentence.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Unless content is expected to be added soon, there is no need to add this file. Consider rm -r doc/developer/interfaces.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[optional] So much for the "Enabling" section. :) Wouldn't it be better to move this sentence to the previous paragraph?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[optional] I would prefer that lines fitted 80 characters. Compare with the formatting of doc/developer/overview.rst.

emacs-long-lines
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[optional] I'm not sure the word "business" is applicable here...

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

The term decree is worth explaining.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[nit,optional] Separate items with semicolon?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[nit,optional] Lowercase sentences and separate them with semicolon. I.e.,s/Spawn/spawn/; s/nodes\./nodes;/. Similarly on lines 66 and 67 (the last sentence ending with dot).

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

[optional] Hyperlink the first occurrence of ‘recovery coordinator’?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

s/additionaly/additionally/

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

The list is not complete.

$ find doc/developer -type f | cut -d/ -f3- | grep -v '^index\.rst$'
components/event-queue.rst
components/recovery-coordinator.rst
components/recovery-supervisor.rst
components/replicator.rst
components/resource-graph.rst
concepts/decision-log.rst
concepts/logging.rst
interfaces/sspl.rst
overview.md
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

Why including itself?

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 7 years ago

Created by: vvv

I'm not sure this is a valid link. AFAICS, it refers to doc/replication/hld.rst, which does not exist.

$ find doc/hld
doc/hld
doc/hld/replication
doc/hld/replication/fig1.png
doc/hld/replication/fig2.png
doc/hld/replication/fig3.png
doc/hld/replication/fig4.png
doc/hld/replication/fig5.png
doc/hld/replication/fig6.png
doc/hld/replication/fig7.png
doc/hld/replication/hld.rst

[optional] Consider moving doc/hld/replication/ to either doc/architecture/ or doc/developer/concepts/ directory and rmdir doc/hld afterwards.

shailesh-vaidya commented 8 months ago

Closing as an obsolete