Skip to content

Conversation

@nikhil550
Copy link
Contributor

@nikhil550 nikhil550 commented Dec 5, 2019

Signed-off-by: NIKHIL E GUPTA [email protected]

Add new test network tutorial as part of getting started section.

Type of change

  • Documentation update

Description

Users would be guided toward the new test network directly after they install the prerequisites and fabric docker images and samples. The tutorial is the first step toward replacing the build a network tutorial in the long term.

Related issues

The PR to add the test network to the Fabric samples:

hyperledger/fabric-samples#80. (Merged)

The tutorial should be merged after the sample

Comment on lines 80 to 82

Choose a reason for hiding this comment

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

This seems misplaced. You're in effect asking people to run a test network when they have no idea what it contains.

I get why --- people shouldn't have to dig around for the script command, but I would at least link to the "components of the network section" somewhere near the beginning so that first time readers can jump down and find out what's in the network they're deploying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to leave as is. People have the notion of a blockchain network before they know what it does. The components overview is the directly section below, so I do not think that there needs to be a link.

Comment on lines 193 to 222

Choose a reason for hiding this comment

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

In general, I think it's a bit backwards to have so much in this paragraph and the one below about "how chaincodes work" rather than their conceptual purpose and role in the network (given comparably little space in the first paragraph). All of this stuff about signatures and executing chaincode on their peer is important, but needs to be understood in the context of smart contracts more in general.

Also worth noting again that we should have links to where some of this stuff is described elsewhere in more detail. I worry that these sections are sort of tweeners --- not going too deep but still trying to give people a good amount of information --- which are incredibly difficult to write. You might find these sections get a lot of scrutiny from maintainers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think mentioning the endorsement policy is important, but it is possible that this section can be cut down. I am going to leave as is for the moment, see what scrutiny I get.

Comment on lines +333 to +365

Choose a reason for hiding this comment

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

These "bring up the network" sections are organized somewhat strangely to me, especially this one after the CLI section above. I might put all of these sections together, either before or after the CLI section, and have links at the top of the doc that can take people down to these "optional deployment" sections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I debated this in terms of putting it above the channel creation, but I wanted to create a friendly experience where users were not hit by too much information to quickly. I will put a link to this section from the top however.

Copy link
Contributor

@guoger guoger left a comment

Choose a reason for hiding this comment

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

just several nits.

Copy link
Contributor

Choose a reason for hiding this comment

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

technically speaking, docker compose does not prohibit it, one could just expose host port and manually add hosts entries. I guess we could say some thing like:

extra configurations are needed to connect other running Fabric nodes, and is out of the scope of this tutorial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rather be even stronger in terms of telling folks that this is not a prod deployment, and not to try to use it to connect to other nodes. How about this:
Because the nodes are isolated within a Docker Compose network, the test network is not configured to connect to other running fabric nodes.

@nikhil550 nikhil550 force-pushed the FAB-17199 branch 2 times, most recently from 982db8d to 6f791d0 Compare December 11, 2019 15:35
@rameshthoomu
Copy link
Contributor

I don't see documentation for adding a new organization using addOrg3.sh script. What's the plan?

@nikhil550
Copy link
Contributor Author

I don't see documentation for adding a new organization using addOrg3.sh script. What's the plan?

We will rebase the adding an org to the network tutorial to the new test network and doc the addOrg3 script there. We will continue to use the byfn tutorial and eyfn until then.

@joealewine
Copy link

Long term the future of EYFN should be to focus on the process for creating the structure and crypto material for an org, and how to add the org to a channel through a channel update (what the json file and jq command would look like, in other words). We can leverage the updated Config Update doc -- https://hyperledger-fabric.readthedocs.io/en/master/config_update.html -- for the latter, rather than that being the focus of an Add an Org tutorial.

Add new test network tutorial as part of
gettign started section

Signed-off-by: NIKHIL E GUPTA <[email protected]>
@denyeart denyeart merged commit 402a077 into hyperledger:master Dec 12, 2019
@nikhil550 nikhil550 deleted the FAB-17199 branch February 3, 2020 15:55
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.

7 participants