Rework readme #32

boguslavsky · 2017-06-12T17:00:38Z

No description provided.

slowli

Comments as of d869456. Nothing criminal, but I feel that there is room for improvement.

slowli · 2017-06-12T19:04:33Z

README.md

@@ -1,858 +1,304 @@
 # Client for Exonum blockchain platform

-JavaScript toolkit to work with Exonum blockchain from both of browser and Node.js.
+This is JavaScript toolkit to work with Exonum blockchain from browser and Node.js.


Nitpick: I think a variant of

exonum-client is a JavaScript toolkit ...

is more popular with READMEs of packages in npm.

slowli · 2017-06-13T14:20:33Z

README.md


-Returns an object of format type.
+Each primitive data type has its own segment length (`size`).


I'd move this paragraph upper, directly after the "field structure" table.

slowli · 2017-06-13T14:21:10Z

README.md


-##### newType.serialize(data)
+Total length of the byte array must be also specified.


IMO, this duplicates the information from the "field structure" table and can be safely removed.

slowli · 2017-06-13T14:22:05Z

README.md


-#### Timespec
+All this operations are useful while work with a blockchain.


Nitpick/grammar: "These operations are useful in working with an Exonum blockchain."

slowli · 2017-06-13T14:23:05Z

README.md


-Unsigned integer value of the length of `8` bytes. Represents Unix time in nanosecond.
+To describe a custom data structure next basic primitive data types can be used:


"To describe" seems pretty vague. I'd say that these types can be used as the type field of the field description.

slowli · 2017-06-13T14:31:05Z

README.md

-var secretKey = '6752be882314f5bbbc9a6af2ae634fc07038584a4a77510ea5eced45f54dc030f5864ab6a5a2190666b47c676bcf15a1f2f07703c5bcafb5749aa735ce8b7c36';
-
-var signature = someType.sign(secretKey, data);
+var secretKey = '07038584a4a77510ea5eced45f54dc030f5864ab6a5a2190666b47c676bcf15a1f2f07703c5bcafb5749aa735ce8b7c366752be882314f5bbbc9a6af2ae634fc';


Nitpick: maybe, split private key/signature into 2 lines to improve readability?

slowli · 2017-06-13T14:33:29Z

README.md


-##### newMessage.serialize(data, cutSignature)
+Field structure:


Maybe, just refer to a section above instead of repeating the table?

slowli · 2017-06-13T14:38:37Z

README.md


---
+Returns random `Uint64` of cryptographic quality as a string.


... as a hexadecimal string.

Actually it is not a hexadecimal string, just number passed as string.

What about:

Returns random `Uint64` of cryptographic quality as a number passed as string.

slowli · 2017-06-13T14:41:31Z

README.md

+
+| Type | Size (bytes) | Description |
+|---|---|---|
+| `string` | `8` | String of UTF-8 characters |


The size of 8 bytes may be confusing.

slowli · 2017-06-13T14:42:48Z

README.md

+
+| Type | Size (bytes) | Description |
+|---|---|---|
+| `Array` | any | Array of 8-bit unsigned integers |


"Size: any" can be confusing, imo. The size is fixed in the field declaration, right? Maybe, write it here explicitly?

boguslavsky · 2017-06-15T15:35:59Z

@slowli ready for review

slowli

Quick comments as of 441acf3.

slowli · 2017-06-15T17:04:12Z

README.md

@@ -35,7 +37,7 @@ Requires a single parameter passed as `object` with next structure:
 | size | `number` | yes | The total length in bytes |
 | fields | `Array` | yes | Array of fields |

-Field structure:
+###### Field structure:


Why use level-6 header here? I'd either remove a header formatting altogether, or use level-4 as implied by the hierarchical structure of the document.

The only reason I've used header here is to have anchor. It is used below in newMessage section:

Field structure is the same as for [custom type](#fieldstructure) defined through `Exonum.newType` method.

Changed it to level-4 header.

slowli · 2017-06-15T17:05:17Z

README.md

---
+| Field | Type | Is mandatory | Description |
+|---|---|---|---|
+| type | built-in primitive type / entity of a `NewClass` | yes | Field type. Can contains fields of a `NewType` types |


NewClass

Do you mean NewType?

slowli · 2017-06-15T17:06:10Z

README.md


-#### newType
+- serialize data into a byte array;


Nitpick: I'd remove blank lines between list items here to get consistent formatting with a list of built-in types below.

Similar nitpicks below.

slowli · 2017-06-15T17:06:39Z

README.md


-##### newType.serialize(data)
+These types can be used as the `type` field of the field description:


Nitpick: "These" -> "The following".

slowli · 2017-06-15T17:07:31Z

README.md

-        name: {type: Exonum.String, size: 8, from: 8, to: 16}
-    }
-});
+And it is necessary to specify segment range in the resulting byte array (`from` and `to`).


Nitpick: remove "And" and unite with the previous sentence into a single paragraph.

slowli · 2017-06-15T17:09:34Z

README.md

-
-var signature = someType.sign(secretKey, data);
+var secretKey = '07038584a4a77510ea5eced45f54dc030f5864ab6a5a2190666b47c676bcf15a
+1f2f07703c5bcafb5749aa735ce8b7c366752be882314f5bbbc9a6af2ae634fc';


By splitting, I meant producing valid JS code:

var secretKey = '07038584a4a77510ea5eced45f54dc030f5864ab6a5a2190666b47c676bcf15a' + '1f2f07703c5bcafb5749aa735ce8b7c366752be882314f5bbbc9a6af2ae634fc';

slowli · 2017-06-15T17:13:06Z

README.md

+    protocol_version: 0,
+    service_id: 0,
+    message_id: 0,
+    signature: '07038584a4a77510ea5eced45f54dc030f5864ab6a5a2190666b47c676bcf15a1f2f07703c5bcafb5749aa735ce8b7c366752be882314f5bbbc9a6af2ae634fc',


Nitpick: a signature here can be split to 2 lines.

slowli · 2017-06-15T17:22:04Z

README.md


 ```
 $ grunt
 ```

-#### Test
+## Test coverage


Sorry, I don't see coverage tasks in Gruntfile.js, and there seemingly aren't any coverage-related dev dependencies (such as istanbul). If you don't mean measuring test coverage here, I think "Testing" could be a better header caption.

boguslavsky · 2017-06-16T13:26:18Z

@slowli changes applied, ready for review

slowli

LGTM as of 66e566c. Probably will need to add a bunch of badges once we go public, but this can wait.

slowli · 2017-06-16T13:56:03Z

README.md

+|---|---|---|
+| `string` | `8`* | String of UTF-8 characters |
+
+*\*Note that the size of 8 bytes is due to the specifics of string [serialization](http://exonum.com/doc/advanced/serialization). Actual string length is not limited.*


Nitpick: well, it's limited by the general message size limits (2 MB, afaik).

slowli · 2017-06-16T14:00:28Z

README.md


 ```
 $ npm install
 ```

-To build minimised and development versions of library execute:
+Compile browser version into `dist` folder:


Comment: technically, the prepublish command (which includes building the browser version) executes during npm install in npm < 5 😃 So, in some cases, this step may be redundant.

Ivan Bohuslavskyi added 3 commits June 12, 2017 19:59

rework readme

c67befe

rework readme #2

6edd9ce

update anchored links in readme

0c38d4f

boguslavsky changed the title ~~WIP: rework readme~~ Rework readme Jun 13, 2017

boguslavsky requested a review from slowli June 13, 2017 11:25

remove Timespec type

d869456

boguslavsky mentioned this pull request Jun 13, 2017

Tracking issue for 0.1 release #14

Closed

9 tasks

slowli reviewed Jun 13, 2017

View reviewed changes

readme fixes

441acf3

slowli reviewed Jun 15, 2017

View reviewed changes

minor readme improvements

66e566c

slowli approved these changes Jun 16, 2017

View reviewed changes

add badges into readme; minor readme updates

7396b8d

boguslavsky merged commit 7ac96b9 into exonum:master Jun 19, 2017

boguslavsky deleted the docs branch June 19, 2017 08:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework readme #32

Rework readme #32

boguslavsky commented Jun 12, 2017

slowli left a comment

slowli Jun 12, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

boguslavsky Jun 15, 2017

boguslavsky Jun 15, 2017

slowli Jun 13, 2017

slowli Jun 13, 2017

boguslavsky commented Jun 15, 2017

slowli left a comment

slowli Jun 15, 2017

boguslavsky Jun 16, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

slowli Jun 15, 2017

boguslavsky commented Jun 16, 2017

slowli left a comment

slowli Jun 16, 2017

slowli Jun 16, 2017


		Returns an object of format type.
		Each primitive data type has its own segment length (`size`).


		##### newType.serialize(data)
		Total length of the byte array must be also specified.


		#### Timespec
		All this operations are useful while work with a blockchain.


		Unsigned integer value of the length of `8` bytes. Represents Unix time in nanosecond.
		To describe a custom data structure next basic primitive data types can be used:


		##### newMessage.serialize(data, cutSignature)
		Field structure:


		---
		Returns random `Uint64` of cryptographic quality as a string.


		##### newType.serialize(data)
		These types can be used as the `type` field of the field description:

Rework readme #32

Rework readme #32

Conversation

boguslavsky commented Jun 12, 2017

slowli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boguslavsky commented Jun 15, 2017

slowli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boguslavsky commented Jun 16, 2017

slowli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment