Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Change getting started structure #2175

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Conversation

neewy
Copy link
Contributor

@neewy neewy commented Mar 15, 2019

[ci skip]

Signed-off-by: Nikolay Yushkevich [email protected]

Description of the Change

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

neewy and others added 2 commits March 15, 2019 16:16
Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

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

This only a part of review

docs/source/getting_started/cli-guide.rst Show resolved Hide resolved
docs/source/getting_started/cli-guide.rst Show resolved Hide resolved
@igor-egorov igor-egorov self-requested a review April 16, 2019 15:02
Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

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

please check the comments

docs/source/getting_started/cli-guide.rst Show resolved Hide resolved
docs/source/getting_started/cli-guide.rst Show resolved Hide resolved
Go back to a terminal where ``irohad`` is running. You can see logs of your
transaction.

Congratulations! You have submitted your first transaction to Iroha.
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many congratulations. Can we replace some of them with .. (idk what will be the best replacement)?

docs/source/getting_started/cli-guide.rst Show resolved Hide resolved
-p 50051:50051 \
-v $(pwd)/iroha/example:/opt/iroha_data \
-v YOUR_PATH_TO_CONF_FILES:/opt/iroha_data \
Copy link
Contributor

Choose a reason for hiding this comment

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

not a good idea to change that line. The reason is - the reader is not able any more do just copy-paste :(

I suggest to revert that line change and still provide an explanation in a section below.

Copy link
Contributor

Choose a reason for hiding this comment

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

true - that's why I added the example below (within the instructions). I think we could leave the option of custom directory somewhere too. Maybe better to leave the previous version here and explain that a custom directory could be used instead further in the instructions?

docs/source/getting_started/python-guide.rst Show resolved Hide resolved
docs/source/getting_started/python-guide.rst Show resolved Hide resolved
Here are Iroha dependancies. Python library generally consists of 3 parts: Iroha, IrohaCrypto and IrohaGrpc which we need to import:

.. code-block:: shell
from iroha.primitive_pb2 import can_set_my_account_detail
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to put in a separate example, after the note, for example

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand, could you please clarify?

docs/source/getting_started/python-guide.rst Show resolved Hide resolved
docs/source/getting_started/python-guide.rst Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants