Format blockchain app #39
Conversation
ghost
assigned 
|
Just noticed that we an issue for some of the blockchain formatting -> #31. I'd be happy to either extract long names into functions so that we can address that issue 31, or I can also close this if we prefer to take a different approach (e.g. a PR per file for formatting blockchain). |
Format blockchain app with elixir formatter. ```elixir mix format --check-equivalent ```
germsvel
force-pushed
the
gv-format-blockchain
branch
from
bdf4ca3 to
902f4b2
Compare
|
@vyorkin I'm definitely in favor of getting this in there, having fixed any glaring issues (like the comments that were fixed in 902f4b2), and then refactor functions gradually. I think fixing all the functions that need to be extracted or going file by file like the elixir core team did sounds nice, but it would add a lot of overhead for the team right now. I'll wait for @ayrat555's thoughts on it before merging since he had tried this as well. But if he's okay with that, I say we go ahead and do what you said. |
| ++ if child_block.header.ommers_hash == block.header.ommers_hash, do: [], else: [:ommers_hash_mismatch] | ||
| ++ if child_block.header.transactions_root == block.header.transactions_root, do: [], else: [:transactions_root_mismatch] | ||
| ++ if child_block.header.receipts_root == block.header.receipts_root, do: [], else: [:receipts_root_mismatch] | ||
| errors = |
There was a problem hiding this comment.
@germsvel that's definitely should be fixed before merging
There was a problem hiding this comment.
Very good catch. This is a disaster in the new format.
There was a problem hiding this comment.
@germsvel it looks much better. good work. You also can add typespecs to new methods.
There was a problem hiding this comment.
Oh good catch. I'll add those and merge after that.
There was a problem hiding this comment.
Added typespecs in 5f22b33
| ] | ||
| defstruct block_hash: nil, | ||
|
|
||
| # Hash for this block, acts simply as a cache. |
Fix formatting on code comments that are describing the elements of a structure relative to an equation found in the yellow paper. The automatic formatting puts any inline comment on a line above the code. When those comments are inline with arguments to a function, it has the strange effect of making it harder to read the function signature and what the comment is referencing. We update three instances where this was the case to make it a single comment above the function signature that explains what all of the arguments are.
The automatic formatting for the holistic validity check in `lib/blockchain/block.ex` was extremely nested since it was doing a lot of `if else` statements and concatenating lists. We update this to extract each validity check to its own private function, and we collect the errors as we go along. One change made here (as will be noted by the change in the doc test) is that instead of concatenating the errors like so, ```elixir [:error1] ++ [:error2] ``` we are adding prepending the latest error (since it's technically faster), ```elixir [:error2 | [:error1]] ``` which has the adverse effect of returning the errors in opposite order (hence the change in the order of the results in the doc tests). If the order of the errors is desired, we can certainly return the errors in that order by reversing the list or doing the checks in reverse order.
germsvel
force-pushed
the
gv-format-blockchain
branch
from
5f22b33 to
d280326
Compare
As part of #33, this PR formats the blockchain app with elixir formatter. We include the
.formatter.exsfile in theapps/blockchain/for now. Once we are done formatting all the apps, we should have a top level.formatter.exsand we can remove the app specific ones.