This repository was archived by the owner on Aug 18, 2020. It is now read-only.
[CDEC-509] Remove HasGenesisHash in favour of explicit parameters #3522
Conversation
This file contains hidden or 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
ruhatch
force-pushed
the
ruhatch/CDEC-509
branch
4 times, most recently
from
c96cef7 to
9ba020c
Compare
erikd
force-pushed
the
ruhatch/CDEC-509
branch
from
9ba020c to
f9a86c0
Compare
erikd
previously requested changes
Sep 5, 2018
ruhatch
force-pushed
the
ruhatch/CDEC-509
branch
from
f9a86c0 to
674df8f
Compare
parsonsmatt
approved these changes
Sep 5, 2018
db/src/Pos/DB/Block/Internal.hs
Outdated
| -> m (Maybe SerializedBlund) | ||
| dbGetSerBlundPureDefault h = do | ||
| dbGetSerBlundPureDefault _ h = do |
There was a problem hiding this comment.
If we're not even using it, why pass it in?
db/src/Pos/DB/Block/Internal.hs
Outdated
| -> m (Maybe SerializedUndo) | ||
| dbGetSerUndoPureDefault h = do | ||
| dbGetSerUndoPureDefault _ h = do |
There was a problem hiding this comment.
Ditto re why pass it in?
| loadHeadersByDepth (k + 1) upToReal | ||
| -- loadHeadersByDepth always returns nonempty list unless you | ||
| -- pass depth 0 (we pass k+1). It throws if upToReal is | ||
| -- absent. So it either throws or returns nonempty. |
There was a problem hiding this comment.
can we encode this in the type? eg make it return NewestFirst NonEmpty BlockHeader?
lib/src/Pos/Launcher/Scenario.hs
Outdated
| runNode coreConfig txpConfig nr plugins = runNode' coreConfig | ||
| nr | ||
| workers' | ||
| plugins |
There was a problem hiding this comment.
This indentation style is too sensitive to changes in lengths of any params or adding/removing any parameters. Instead, prefer something like:
runNode coreConfig txpConfig nr plugs =
runNode'
coreConfig
nr
workers'
pluginsThen a change to a parameter name or addition of a parameter will only affect the relevant line.
There was a problem hiding this comment.
Yeah, good point! I'm using Brittany at the moment, so I'll look into a way to remove that kind of indentation...
ruhatch
force-pushed
the
ruhatch/CDEC-509
branch
from
674df8f to
334944b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
Almost there with the
Data.Reflectionconstraints fromcore.Linked issue
CDEC-509
Type of change
Developer checklist
Testing checklist
QA Steps
Screenshots (if available)