Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation, particularly for FSM Module #8

Merged
merged 3 commits into from
Mar 14, 2020

Conversation

hannahhoward
Copy link
Contributor

@hannahhoward hannahhoward commented Mar 6, 2020

Goals

Provide basic module documentation since that is a source of confusion
Provide guidance on using FSM module beyond examples in test

Implementation

  • Add a detailed README for FSM module cause I know it best
  • Add a simple README for the generic state machine module since it's in use in go-storage-miner

Add a readme to fsm module outlining usage
@hannahhoward hannahhoward force-pushed the feat/add-readme-fsm branch from f821f0b to 10a7c9d Compare March 6, 2020 00:24
add general readme information and a few corrections to fsm docs
@hannahhoward hannahhoward changed the title README to document FSM Module module use Documentation, particularly for FSM Module Mar 6, 2020
README.md Outdated
It has the following signature:

```golang
type Planner func(events []Event, user interface{}) (interface{}, uint64, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Give names to the return parameters.

README.md Outdated
type Planner func(events []Event, user interface{}) (interface{}, uint64, error)
```

A planner receives a series of events and a point to the current state (represented by user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and a point to the current state

Suggestion: "and a pointer to the current state"

fsm/README.md Outdated

A state machine is defined in terms of

- StateType -- the type of data structure we are tracking (should be a struct)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Some things in this list, e.g. Events, are backtick'ed - and others (like StateKeyField) are not.

fsm/README.md Outdated
Let's pretend our ideal deal flow looks like:

```
Receive new deal proposal -> Validate proposal-> For All Data Requested, Send a chunk, then request payment, then wait for payment before sending more -> Complete deal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

Change

Validate proposal->

to

Validate proposal ->

Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super useful!

README.md Outdated
type Planner func(events []Event, user interface{}) (interface{}, uint64, error)
```

A planner receives a series of events and a point to the current state (represented by user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pointer"?

README.md Outdated
You can now dispatch events to a state machine with:

```golang
var id interface{} // some identifier of a deal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be identifier of a "tracked state" or something like that.

fsm/README.md Outdated

## Usage

Let's consider a hypothetical deal we want to track in Filecoin. Each deal will have a series of states it can be in, and various things that can happen to change its state. We will track multiple deals at the same time. In this example, we will model the actions of the receiving party (i.e. the person who accepted the deal and responding)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the person who accepted the deal and is responding

fsm/README.md Outdated
ReceivedNewDeal
AcceptedDeal
RejectedDeal
SentBlocks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be SentData to be consistent with below...

@hannahhoward hannahhoward merged commit 2e7830f into master Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants