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

Add components RFC #9

Open
wants to merge 1 commit into
base: master
from

Conversation

@vaporos
Copy link
Member

commented May 6, 2019

Proposes the initial high-level structure of the project and the layout
of the source code.

Signed-off-by: Shawn T. Amundson amundson@bitwise.io

Proposes the initial high-level structure of the project and the layout
of the source code.

Signed-off-by: Shawn T. Amundson <amundson@bitwise.io>
@vaporos vaporos requested review from davececchi and dcmiddle as code owners May 6, 2019
@vaporos vaporos self-assigned this May 6, 2019
@agunde406 agunde406 requested review from agunde406 and removed request for agunde406 May 6, 2019
`grid/cli/` - The Grid CLI command source code.

`grid/contracts/<contract>/` - Smart contract source code, with each smart
contract having a separate directory “<contract>”.

This comment has been minimized.

Copy link
@agunde406

agunde406 May 14, 2019

Contributor

<contract> has weird formatting. try using ` instead of ”

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

Markdown (and Sphinx) don't generally let you mix fixed-width and italic fonts in the same string. The docs use { curly braces } for this reason (not < angle brackets > ).

I'd also suggest using contract-name instead of contract. But that's a nitpick.

Copy link
Contributor

left a comment

In general, this RFC is clear and has great info. I've commented on the confusing and contradictory items. I also marked some errors and suggested a few wording improvements.

@@ -0,0 +1,261 @@
- Feature Name: components

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

The feature name doesn't match the contents of this RFC.

The first paragraph says "This RFC proposes the initial
high-level structure of the project and the layout of the source code." But that doesn't appear until much later (under "Reference-level Explanation").

# Summary

Hyperledger Grid is a platform which contains various components which can be
assembled to implement supply chain solutions. This RFC proposes the initial

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

First, this RFC describes several Grid components (gridd daemon, grid command, Grid SDK, smart contracts, and documentation).

backwards compatibility should be considered. The SDK’s versioning should
conform to semver (which is true of all components, but of particular
importance for the SDK).
Smart Contracts

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

Need a space before this line.


Grid contains the following components:

- A daemon process called “gridd” (pronounced grid-dee) or Grid daemon

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

Is "Grid daemon" an alternate pronunciation or a different name under consideration? I suspect it's the former, so:

Change: "grid-dee) or Grid daemon"

to: "grid-dee or "Grid daemon)"


Hyperledger Grid is a platform which contains various components which can be
assembled to implement supply chain solutions. This RFC proposes the initial
high-level structure of the project and the layout of the source code.

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

Need to clarify that this is the project structure and source-code layout for the hyperledger/grid repo (not hyperledger/grid-contrib).

we have chosen formats for documentation which are friendly to source control
under git while providing the best output possible. The documentation is
written in: a) Rust doc comments for SDK API documentation; b) sphinx-doc for
the bulk of non-API documentation; c) Markdown friendly for viewing directly in

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

Is there a flavor of Markdown called "Markdown friendly"? I've never heard of it, and couldn't find it with a google search.

I suspect this is actually GitHub Flavored Markdown (GFM). If so, it would be helpful to state that, because it can take a newbie ages to figure out why "standard" Markdown doesn't render correctly in a github README.

Here's the link this noob discovered after searching (for ages):
https://help.github.com/en/categories/writing-on-github

the bulk of non-API documentation; c) Markdown friendly for viewing directly in
github for specific other documents.

The sphinx-doc documentation will include an introduction to Grid, user guides,

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

s/sphinx-doc/RST/


As new additions to Grid are added the documentation should be updated as well.

# Reference-level explanation

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

This title is confusing as the "name" of this particular section. This section explains the repo/source code layout, which (I suppose) could be considered implementation information. But it doesn't have any implementation info for the preceding sections in this document.

The Grid CLI may not be necessary for all users of Grid as many of its
functions could be done by programming against the REST API.

The Grid Daemon process will grow over time to include many components and

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

s/Daemon/daemon/


`grid/daemon/` - The Grid daemon source code.

`grid/docs/` - The sphinx-doc documentation source code.

This comment has been minimized.

Copy link
@chenette

chenette May 14, 2019

Contributor

s/sphinx-doc/RST/

@chenette chenette self-requested a review May 14, 2019
Copy link
Contributor

left a comment

(Second try at clicking the Request changes button.)

The Grid daemon process contains all of the components which run as
a persistent process as part of Hyperledger Grid. An intentional part of the
Grid architecture is to minimize persistent processes to a bare minimum with
the ideal state being a single Grid daemon process. This is architecturally

This comment has been minimized.

Copy link
@desktophero

desktophero Aug 15, 2019

is a single Grid daemon process and the associated state at an individual node/instance level? My thought is there may be more than instance (scale, maintenance, rolling upgrade, etc.). Would any other instances in a "cluster" of Grid nodes be similar to read-only replicas with state handled by a main/primary? Or maybe that's a future concept.

provided by Hyperledger Grid. This includes additional APIs for data-related
read/query, Grid management and configuration (potentially including management
and configuration of the Grid daemon itself), application key “escrow”, and any
other functionality which is provided by Grid for applications.

This comment has been minimized.

Copy link
@desktophero

desktophero Aug 15, 2019

For clarity, are write methods not going to be exposed through the Grid daemon ReST API (outside of management opportunities called out).

For example, the ability to modify schema data is a separate concern and bounded context.

Asking as perhaps that should be declared here with pointers to where these types of activities will live.

be checked in a bash script. The command’s output should be both human friendly
and easily parsable with common Linux command line tools such as awk and sed
(when necessary, as different command-line options).

This comment has been minimized.

Copy link
@desktophero

desktophero Aug 15, 2019

Any value in getting into things like CLI output format expectations, like --output=yaml for human-friendly views, output=doc for table view, and output=json for programmatic or logging?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.