Skip to content
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

Use Haskell variables for passing values. #5011

Merged
merged 1 commit into from
Mar 29, 2023
Merged

Conversation

MarcFontaine
Copy link
Contributor

Currently the cardano-testnet uses filenames (Strings) to reference objects like keys, certs etc.
Two locations in the code refer to the same object by using the same filename.
This means that the data flow is implicit and hard to follow.
Old scheme:

do
cli_call_1 "filenameX" "filenameY"
cli_call_2 "filenameX" "filenameZ"
cli_call_3 "filenameA" "filenameB"

This PR changes the design as follows:

  • File names are stored in an opaque File datatype (which wraps the file path).
  • A wrapper is used for CLI calls. When a CLI call creates a file, the wrapper returns the corresponding File resource.
  • While CLI calls have type IO () the wrapper has type `IO (File,..,File).
  • CLI calls don't construct Filenames (i.e. as magic constants) , but instead use the File resource that was created earlier.

The new scheme is

do
(file1, file2) <- cli_call1 arg1 arg2     -- cli_call1 creates file resources file1 and file2
(fileX, fileY) <- cli_call2 file1 file2    -- cli_call2 uses file resourcs file1 and file2

Full transformation to the new scheme for filenames is still WIP.

@MarcFontaine
Copy link
Contributor Author

This is a stripped down version of #4845.
This PR introduces CLI wrapper functions and changes three CLI calls to use the wrappers.
Transitioning all CLI calls in cardano-testnet to wrapper functions needs to be done step by step
and combined with other changes. (To be done in followup PRs.)

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice improvement in terms of size of the PR. I left a few comments that need to be address. In future, do not include function definitions that are not in used in the PR.

cardano-testnet/src/Testnet/Shelley.hs Outdated Show resolved Hide resolved
cardano-testnet/src/Testnet/Util/Cli.hs Outdated Show resolved Hide resolved
@MarcFontaine MarcFontaine requested a review from Jimbo4350 March 24, 2023 15:20
Use the wrapper functions in the Shelley testnet.
@MarcFontaine MarcFontaine enabled auto-merge March 29, 2023 12:04
@MarcFontaine MarcFontaine added this pull request to the merge queue Mar 29, 2023
Merged via the queue into master with commit 7b249f6 Mar 29, 2023
@iohk-bors iohk-bors bot deleted the mafo/cli-wrappers branch March 29, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants