Reorganise ouroboros-consensus #1657

mrBliss · 2020-02-18T11:14:25Z

Split off Byron related code and tests to ./ouroboros-consensus-byron.
Split off Byron Spec related code and tests to
./ouroboros-consensus-byronspec.
Split off mock block related code and tests to
./ouroboros-consensus/ouroboros-consensus-mock.
Split off test infrastructure code and tests to
./ouroboros-consensus/ouroboros-consensus-test-infra.

These last two are both internal libraries of ouroboros-consensus and
separate packages, because the test suites of ouroboros-consensus depend on
them, while other packages, i.e., ouroboros-consensus-byron, cardano-node,
..., will also depend on them. If only Cabal supported multiple public
libraries in one project.

Remove the Exception instance of PBftLeaderCredentialsError, as it was
never actually thrown.
Add nodeToExitReason, as it can be blk-dependent.
Rename Ouroboros.Storage to Ouroboros.Consensus.Storage.

ouroboros-consensus/test-consensus/Test/Consensus/Protocol/PBFT.hs

ouroboros-consensus/test-consensus/Test/Consensus/BlockchainTime/SlotLengths.hs

edsko · 2020-02-18T11:27:20Z

While we're at it (can be separate PR): ChainSync.Client and ChainSync.Server?

edsko · 2020-02-18T11:34:56Z

The -byron package should probably have a different module hiearchy: Ouroboros.Consensus.Byron.*? Otherwise people like @mrBliss might start complaining it's hard to locate modules :)

edsko · 2020-02-18T13:40:55Z

Moving Ouroboros.Storage to Ouroboros.Consensus.Storage, but we should probably flatten this a bit:

Merge Ouroboros.Consensus.Util and Ouroboros.Consensus.Storage.Util
Move up Ouroboros.Consensus.Storage.Common
Move up some other modules (EpochInfo for instance)?

ouroboros-consensus-byron/src/Ouroboros/Consensus/Byron/Protocol/PBFT.hs

ouroboros-consensus/README.md

ouroboros-consensus/ouroboros-consensus-mock/README.md

ouroboros-consensus/ouroboros-consensus-mock/ouroboros-consensus-mock.cabal

ouroboros-consensus-byron/ouroboros-consensus-byron.cabal

ouroboros-consensus-cardano/ouroboros-consensus-cardano.cabal

ouroboros-consensus/src/Ouroboros/Consensus/Node/Run.hs

ouroboros-consensus/src/Ouroboros/Consensus/Protocol/PBFT/Crypto.hs

edsko

Reviewed over hangout. And I thought I was bad when it came to refactoring 😁 Good stuff, will be great to get this in. Then more refactoring coming up soon :)

Fixes #1651 and #1652. * Split off Byron related code and tests to `./ouroboros-consensus-byron`. * Split off Byron Spec related code and tests to `./ouroboros-consensus-byronspec`. * Split off mock block related code and tests to `./ouroboros-consensus/ouroboros-consensus-mock`. * Split off test infrastructure code and tests to `./ouroboros-consensus/ouroboros-consensus-test-infra`. These last two are both internal libraries of `ouroboros-consensus` and separate packages, because the test suites of `ouroboros-consensus` depend on them, while other packages, i.e., `ouroboros-consensus-byron`, `cardano-node`, ..., will also depend on them. If only Cabal supported multiple public libraries in one project. * Remove the `Exception` instance of `PBftLeaderCredentialsError`, as it was never actually thrown. * Add `nodeToExitReason`, as it can be `blk`-dependent. * Rename `Ouroboros.Storage` to `Ouroboros.Consensus.Storage`.

Running `cabal test all` should not run the tests of `cardano-ledger`, etc.

If we disable it now without having a nightly job to run it, it will bitrot for sure. You can still disable the test locally by disabling the `cddl` flag in your `cabal.project.local`.

mrBliss · 2020-02-19T09:09:18Z

bors r+

1657: Reorganise ouroboros-consensus r=mrBliss a=mrBliss Fixes #1651 and #1652. * Split off Byron related code and tests to `./ouroboros-consensus-byron`. * Split off Byron Spec related code and tests to `./ouroboros-consensus-byronspec`. * Split off mock block related code and tests to `./ouroboros-consensus/ouroboros-consensus-mock`. * Split off test infrastructure code and tests to `./ouroboros-consensus/ouroboros-consensus-test-infra`. These last two are both internal libraries of `ouroboros-consensus` and separate packages, because the test suites of `ouroboros-consensus` depend on them, while other packages, i.e., `ouroboros-consensus-byron`, `cardano-node`, ..., will also depend on them. If only Cabal supported multiple public libraries in one project. * Remove the `Exception` instance of `PBftLeaderCredentialsError`, as it was never actually thrown. * Add `nodeToExitReason`, as it can be `blk`-dependent. * Rename `Ouroboros.Storage` to `Ouroboros.Consensus.Storage`. Co-authored-by: Thomas Winant <thomas@well-typed.com>

iohk-bors · 2020-02-19T09:29:32Z

Build succeeded

After IntersectMBO/ouroboros-network#1657

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

588: Update ouroboros-network dependency r=mrBliss a=mrBliss Issue ------ Pushing through the changes made in IntersectMBO/ouroboros-network#1657 checklist --------- - [ ] This PR contains all the work required to resolve the linked issue. - [ ] This PR results in breaking changes to upstream dependencies. - [ ] The work contained has sufficient documentation to describe what it does and how to do it. - [ ] The work has sufficient tests and/or testing. - [ ] I have committed clear and descriptive commits. Be considerate as somebody else will have to read these. - [ ] I have added the appropriate labels to this PR. Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Marcin Szamotulski <profunctor@pm.me>

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

588: Update ouroboros-network dependency r=mrBliss a=mrBliss Issue ------ Pushing through the changes made in IntersectMBO/ouroboros-network#1657 checklist --------- - [ ] This PR contains all the work required to resolve the linked issue. - [ ] This PR results in breaking changes to upstream dependencies. - [ ] The work contained has sufficient documentation to describe what it does and how to do it. - [ ] The work has sufficient tests and/or testing. - [ ] I have committed clear and descriptive commits. Be considerate as somebody else will have to read these. - [ ] I have added the appropriate labels to this PR. Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Marcin Szamotulski <profunctor@pm.me> Co-authored-by: Luke Nadur <19835357+intricate@users.noreply.github.com>

mrBliss added the consensus issues related to ouroboros-consensus label Feb 18, 2020

mrBliss requested review from edsko and nfrisby February 18, 2020 11:14

mrBliss force-pushed the mrBliss/reorganise-consensus branch from 2395572 to 0afe404 Compare February 18, 2020 11:16

mrBliss linked an issue Feb 18, 2020 that may be closed by this pull request

Separate out Byron and Shelley integration from consensus #1652

Closed

mrBliss mentioned this pull request Feb 18, 2020

Shelley integration #1557

Closed

mrBliss commented Feb 18, 2020

View reviewed changes

ouroboros-consensus/test-consensus/Test/Consensus/Protocol/PBFT.hs Outdated Show resolved Hide resolved

ouroboros-consensus/test-consensus/Test/Consensus/BlockchainTime/SlotLengths.hs Show resolved Hide resolved

mrBliss force-pushed the mrBliss/reorganise-consensus branch from 0afe404 to 5833f6b Compare February 18, 2020 12:32

mrBliss force-pushed the mrBliss/reorganise-consensus branch 4 times, most recently from 3b3dc7f to c37cbfd Compare February 18, 2020 16:06

mrBliss mentioned this pull request Feb 18, 2020

test-consenus: update README #1478

Closed

mrBliss commented Feb 18, 2020

View reviewed changes

edsko reviewed Feb 18, 2020

View reviewed changes

edsko approved these changes Feb 18, 2020

View reviewed changes

mrBliss force-pushed the mrBliss/reorganise-consensus branch from c37cbfd to 8be6a2b Compare February 18, 2020 17:58

mrBliss added 3 commits February 18, 2020 19:51

Disable the tests for the source-repository-package dependencies

487c8d7

Running `cabal test all` should not run the tests of `cardano-ledger`, etc.

Add missing else to ouroboros-network.cabal

56e5331

mrBliss force-pushed the mrBliss/reorganise-consensus branch from 5c6f693 to 56e5331 Compare February 18, 2020 18:51

mrBliss added 2 commits February 19, 2020 09:06

Use shared directories instead of internal cabal libraries

db23511

Build the CDDL test, as CI depends on it

eba39ad

If we disable it now without having a nightly job to run it, it will bitrot for sure. You can still disable the test locally by disabling the `cddl` flag in your `cabal.project.local`.

iohk-bors bot merged commit eba39ad into master Feb 19, 2020

iohk-bors bot deleted the mrBliss/reorganise-consensus branch February 19, 2020 09:29

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 19, 2020

Update ouroboros-network dependency

d6fda5c

After IntersectMBO/ouroboros-network#1657

mrBliss mentioned this pull request Feb 19, 2020

Update ouroboros-network dependency IntersectMBO/cardano-node#588

Merged

6 tasks

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 19, 2020

Update ouroboros-network dependency

2f694b0

After IntersectMBO/ouroboros-network#1657

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 19, 2020

Update ouroboros-network dependency

ff0b62d

After IntersectMBO/ouroboros-network#1657

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 19, 2020

Update ouroboros-network dependency

29f02eb

After IntersectMBO/ouroboros-network#1657

mrBliss mentioned this pull request Feb 20, 2020

Cardano client library #1676

Closed

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 20, 2020

Update ouroboros-network dependency

8fb22e6

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Feb 25, 2020

Update ouroboros-network dependency

8fe6208

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

intricate pushed a commit to IntersectMBO/cardano-node that referenced this pull request Feb 26, 2020

Update ouroboros-network dependency

3fbff22

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

intricate pushed a commit to IntersectMBO/cardano-node that referenced this pull request Feb 26, 2020

Update ouroboros-network dependency

5395285

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

intricate pushed a commit to IntersectMBO/cardano-node that referenced this pull request Feb 26, 2020

Update ouroboros-network dependency

654d0a9

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

intricate pushed a commit to IntersectMBO/cardano-node that referenced this pull request Feb 26, 2020

Update ouroboros-network dependency

4fe75fd

After IntersectMBO/ouroboros-network#1657 Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganise ouroboros-consensus #1657

Reorganise ouroboros-consensus #1657

mrBliss commented Feb 18, 2020 •

edited

Loading

edsko commented Feb 18, 2020 •

edited

Loading

edsko commented Feb 18, 2020

edsko commented Feb 18, 2020

edsko left a comment

mrBliss commented Feb 19, 2020

iohk-bors bot commented Feb 19, 2020

Reorganise ouroboros-consensus #1657

Reorganise ouroboros-consensus #1657

Conversation

mrBliss commented Feb 18, 2020 • edited Loading

edsko commented Feb 18, 2020 • edited Loading

edsko commented Feb 18, 2020

edsko commented Feb 18, 2020

edsko left a comment

Choose a reason for hiding this comment

mrBliss commented Feb 19, 2020

iohk-bors bot commented Feb 19, 2020

Build succeeded

mrBliss commented Feb 18, 2020 •

edited

Loading

edsko commented Feb 18, 2020 •

edited

Loading