doc: fix markdown for the decimal package #202
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ligurio
reviewed
Aug 4, 2022
decimal/decimal.go
Outdated
Comment on lines
3
to
39
| // The package implements methods to encode and decode BCD. | ||
| // | ||
| // BCD (Binary-Coded Decimal) is a sequence of bytes representing decimal | ||
| // digits of the encoded number (each byte has two decimal digits each encoded | ||
| // using 4-bit nibbles), so byte >> 4 is the first digit and byte & 0x0f is the | ||
| // second digit. The leftmost digit in the array is the most significant. The | ||
| // rightmost digit in the array is the least significant. | ||
| // | ||
| // The first byte of the BCD array contains the first digit of the number, | ||
| // represented as follows: | ||
| // | ||
| // | 4 bits | 4 bits | | ||
| // = 0x = the 1st digit | ||
| // | ||
| // (The first nibble contains 0 if the decimal number has an even number of | ||
| // digits). The last byte of the BCD array contains the last digit of the | ||
| // number and the final nibble, represented as follows: | ||
| // | ||
| // | 4 bits | 4 bits | | ||
| // = the last digit = nibble | ||
| // | ||
| // The final nibble represents the number's sign: 0x0a, 0x0c, 0x0e, 0x0f stand | ||
| // for plus, 0x0b and 0x0d stand for minus. | ||
| // | ||
| // Examples: | ||
| // | ||
| // The decimal -12.34 will be encoded as 0xd6, 0x01, 0x02, 0x01, 0x23, 0x4d: | ||
| // | ||
| // | MP_EXT (fixext 4) | MP_DECIMAL | scale | 1 | 2,3 | 4 (minus) | | ||
| // | 0xd6 | 0x01 | 0x02 | 0x01 | 0x23 | 0x4d | | ||
| // | ||
| // The decimal 0.000000000000000000000000000000000010 will be encoded as | ||
| // 0xc7, 0x03, 0x01, 0x24, 0x01, 0x0c: | ||
| // | ||
| // | MP_EXT (ext 8) | length | MP_DECIMAL | scale | 1 | 0 (plus) | | ||
| // | 0xc7 | 0x03 | 0x01 | 0x24 | 0x01 | 0x0c | | ||
| // |
There was a problem hiding this comment.
This comment about BCD encoding/decoding was placed to a file bcd.go intentionally.
Sure, you can move it to another place, although I don't get your motivation.
Additionally, this change is not related to the fixed ticket, why it was added to commit with fix?
There was a problem hiding this comment.
The change related to the second part of the commit: "It also makes the package description less confusing, removes duplication."
It fixes general description of the decimal subpackage:
I can split the commit into two parts, but that looks redundant to me.
There was a problem hiding this comment.
Perhaps the description of the internal implementation is redundant in the description of the package. I'm hesitant to turn it into a code comment.
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
from
520bb71 to
e9f45ab
Compare
|
I've moved the implementation details from GoDoc comments to the code comments. |
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
2 times, most recently
from
ff497a2 to
ba1aa4b
Compare
oleg-jukovec
requested review from
DifferentialOrange
and removed request for
AnaNek
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
2 times, most recently
from
1283d64 to
a820c8d
Compare
oleg-jukovec
changed the title
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
from
a820c8d to
89896c3
Compare
oleg-jukovec
changed the title
Merged
DifferentialOrange
approved these changes
Aug 17, 2022
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
from
89896c3 to
9ee249f
Compare
The patch hides the implementation details of the Decimal type from documentation. It provides comments for Decimal.MarshalMsgpack and Decimal.UnmarshalMsgpack. Closes #201
oleg-jukovec
force-pushed
the
oleg-jukovec/gh-201-fix-decimal-godoc
branch
from
9ee249f to
b9ddb95
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The patch hides the implementation details of the Decimal type from
documentation. It provides comments for Decimal.MarshalMsgpack and
Decimal.UnmarshalMsgpack.
How-to test:
$ godoc -http=:8080in the go-tarantool folder.I didn't forget about (remove if it is not applicable):
Related issues:
Closes #201