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

cardano-testnet: QoL changes #6008

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Conversation

smelc
Copy link
Contributor

@smelc smelc commented Oct 10, 2024

[edit] QoL improvements, because most of this PR's content has been concurrently implemented in #6007

@smelc smelc changed the title cardano-testnet: allow parameters passed to start the nodes cardano-testnet: honor custom parameters passed to start the nodes Oct 10, 2024
@smelc smelc force-pushed the smelc/testnet-honor-nodes-custom-params branch from 5a845ea to b87724e Compare October 10, 2024 14:40
@smelc smelc marked this pull request as ready for review October 10, 2024 14:42
@smelc smelc requested a review from a team as a code owner October 10, 2024 14:42
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.

One minor comment.

@@ -65,7 +65,7 @@ pCardanoTestnetCliOptions envCli = CardanoTestnetOptions
pNumSpoNodes :: Parser [TestnetNodeOptions]
pNumSpoNodes =
OA.option
((`L.replicate` SpoTestnetNodeOptions Nothing []) <$> auto)
((`L.replicate` TestnetNodeOptions Nothing []) <$> auto)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep the differentiation between SPOs and relays.

Base automatically changed from smelc/testnet-remove-unused-datatype to master October 11, 2024 08:54
@smelc
Copy link
Contributor Author

smelc commented Oct 14, 2024

The point of this PR was actually implemented in #6007, so this one becomes mostly empty.

@smelc smelc force-pushed the smelc/testnet-honor-nodes-custom-params branch 2 times, most recently from e0b10a1 to 0f46d78 Compare October 14, 2024 09:37
@smelc smelc changed the title cardano-testnet: honor custom parameters passed to start the nodes cardano-testnet: QoL changes Oct 14, 2024
@@ -65,6 +65,8 @@ pCardanoTestnetCliOptions envCli = CardanoTestnetOptions

pNumSpoNodes :: Parser [TestnetNodeOptions]
pNumSpoNodes =
-- We don't support passing custom node configurations files on the CLI.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -144,7 +144,7 @@ getDefaultShelleyGenesis asbe maxSupply opts = do
-- > │   └── README.md
-- > ├── drep-keys
-- > │   ├── drep{1,2,3}
-- > │   │   └── drep.{skey,drep.vkey}
-- > │   │   └── drep.{skey,vkey}
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you notice to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's @carbolymer who did this change, I think after seeing my (late) review on #6007 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

@smelc smelc force-pushed the smelc/testnet-honor-nodes-custom-params branch from 2c8a630 to 7478ef1 Compare October 14, 2024 14:07
@smelc smelc enabled auto-merge October 14, 2024 14:07
@smelc smelc force-pushed the smelc/testnet-honor-nodes-custom-params branch from 7478ef1 to 37fcfaa Compare October 14, 2024 19:52
@smelc smelc force-pushed the smelc/testnet-honor-nodes-custom-params branch from 37fcfaa to 31cd564 Compare October 15, 2024 10:14
@smelc smelc added this pull request to the merge queue Oct 17, 2024
Merged via the queue into master with commit 5f73911 Oct 17, 2024
23 checks passed
@smelc smelc deleted the smelc/testnet-honor-nodes-custom-params branch October 17, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants