-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove references and logic of system property besu.docker #808
Remove references and logic of system property besu.docker #808
Conversation
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1960,42 +1954,19 @@ private String genesisConfig() { | |||
} | |||
|
|||
private File genesisFile() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem to be much benefit in wrapping this in a function
} else { | ||
return getDefaultBesuDataPath(this); | ||
} | ||
return standaloneCommands.dataPath.toAbsolutePath(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, think we can inline this function where it is needed
@@ -991,9 +986,7 @@ void setBesuConfiguration(final BesuConfiguration pluginCommonConfiguration) { | |||
|
|||
private BesuCommand handleStandaloneCommand() { | |||
standaloneCommands = new StandaloneCommand(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is folding StandaloneCommand back into Besu Command part of this PR or a follow on PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that bit, I've added it into same PR.
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
…k into BesuCommand Signed-off-by: Usman Saleem <usman@usmans.info>
…perty Signed-off-by: Usman Saleem <usman@usmans.info>
PR description
BesuCommand class (and related unit tests) has quite a bit of deprecated logic which sets various defaults depending on if system property besu.docker is present or not. However, this property is not set in Besu's docker file anymore. Hence, the relevant code needs to be removed.
Fixed Issue(s)
#785
Signed-off-by: Usman Saleem <usman@usmans.info>