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

Rename TestEnableDevelopmentHardForkEras and TestEnableDevelopmentNetworkProtocols #4341

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

LudvikGalois
Copy link
Contributor

@LudvikGalois LudvikGalois commented Aug 16, 2022

TestEnableDevelopmentHardForkEras has been renamed to
ExperimentalHardForksEnabled . An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

TestEnableDevelopmentNetworkProtocols has been renamed to
ExperimentalProtocolsEnabled. An error is thrown if
TestEnableDevelopmentNetworkProtocols is used to avoid it silently being set
to False.

Closes #4043
Closes #4470

@newhoggy
Copy link
Contributor

It won't be enough to just change the code. There are some tests that use the TestEnableDevelopmentHardForkEras configuration that now need to do something else. Grep over the code to find these occurrences.

@LudvikGalois LudvikGalois force-pushed the ludvikgalois/testenableadvertiseprotver branch from ce9fcf3 to 9146c99 Compare August 23, 2022 05:09
@LudvikGalois
Copy link
Contributor Author

It won't be enough to just change the code. There are some tests that use the TestEnableDevelopmentHardForkEras configuration that now need to do something else. Grep over the code to find these occurrences.

Fixed

@LudvikGalois LudvikGalois force-pushed the ludvikgalois/testenableadvertiseprotver branch from 9146c99 to de2ec58 Compare August 25, 2022 02:45
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 change

cardano-node/src/Cardano/Node/Configuration/POM.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Protocol/Cardano.hs Outdated Show resolved Hide resolved
@LudvikGalois LudvikGalois force-pushed the ludvikgalois/testenableadvertiseprotver branch from de2ec58 to c4b4fc8 Compare October 24, 2022 06:15
@LudvikGalois LudvikGalois force-pushed the ludvikgalois/testenableadvertiseprotver branch 2 times, most recently from d0ea45e to d47eef1 Compare October 31, 2022 13:01
@newhoggy
Copy link
Contributor

In the diff, I see renames to TestEnableAdvertiseDevelopmentProtVer, but in the PR description, and change log it notes renames to TestEnableDevelopmentProtVer.

Can I confirm the code changes are correct and the change log and PR description need to be updated to match the code?

@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch from d47eef1 to fd284b8 Compare February 27, 2023 13:13
@newhoggy newhoggy dismissed Jimbo4350’s stale review February 27, 2023 13:14

Rebase and updated

@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch from fd284b8 to 1e3563c Compare March 22, 2023 23:23
@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch 5 times, most recently from f34f566 to 71c5e9d Compare April 6, 2023 03:16
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.

LGTM!

@newhoggy newhoggy enabled auto-merge April 7, 2023 01:01
@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch from 71c5e9d to 234b072 Compare April 10, 2023 04:16
@newhoggy newhoggy disabled auto-merge April 10, 2023 04:16
@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch from 234b072 to 8ab8d91 Compare April 11, 2023 01:53
@newhoggy newhoggy changed the title Rename TestEnableDevelopmentHardForkEras Rename TestEnableDevelopmentHardForkEras and TestEnableDevelopmentNetworkProtocols Apr 11, 2023
TestEnableDevelopmentHardForkEras has been renamed to
TestEnableDevelopmentProtVer. An error is thrown if
TestEnableDevelopmentHardForkEras is used to avoid it silently being set
to False.

Closes #4043
@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch 4 times, most recently from 1f04646 to eb8ac21 Compare April 11, 2023 05:35
TestEnableDevelopmentNetworkProtocols has been renamed to
ExperimentalProtocols. An error is thrown if
TestEnableDevelopmentNetworkProtocols is used to avoid it silently being set
to False.
@newhoggy newhoggy force-pushed the ludvikgalois/testenableadvertiseprotver branch from eb8ac21 to d2cd5c4 Compare April 11, 2023 05:41
@newhoggy newhoggy added this pull request to the merge queue Apr 11, 2023
Merged via the queue into master with commit ca78f37 Apr 11, 2023
@iohk-bors iohk-bors bot deleted the ludvikgalois/testenableadvertiseprotver branch April 11, 2023 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants