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

Update dependencies on consensus and ledger #1465

Merged
merged 2 commits into from
Jul 15, 2020
Merged

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented Jul 15, 2020

A couple tracers gained some extra warning fields. Log the new ledger
warnings

Only properly supported for the JSON logging, not for the human
readable, since I can't quite see how it's supposed to be done without
HasTextFormatter constaints propagating everywhere, which does not seem
to be the intended pattern.

@dcoutts
Copy link
Contributor Author

dcoutts commented Jul 15, 2020

@CodiePP perhaps you can enlighten me.

@dcoutts
Copy link
Contributor Author

dcoutts commented Jul 15, 2020

@mrBliss This also depends on a one-liner in consensus

+++ b/ouroboros-consensus/src/Ouroboros/Consensus/HardFork/Combinator/Ledger.hs
@@ -20,6 +20,7 @@
 module Ouroboros.Consensus.HardFork.Combinator.Ledger (
     HardForkLedgerError(..)
   , HardForkEnvelopeErr(..)
+  , HardForkLedgerWarning(..)
     -- * Type family instances
   , Ticked(..)
     -- * Low-level API (exported for the benefit of testing)

@@ -168,78 +168,78 @@ source-repository-package
source-repository-package
type: git
location: https://github.com/input-output-hk/cardano-ledger-specs
tag: 6611345670dad971fbd03594a00b5ef8afa4cae6
--sha256: 0rhdlf7kz62231127ywbakhsgsxhhkba5gz24dfy7l9r45a1ijkk
tag: 47e2075f751d8370222a2aad32f37699a341ec40
Copy link
Contributor

Choose a reason for hiding this comment

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

I was planning to update the dependency on cardano-ledger-specs today, now that IntersectMBO/cardano-ledger#1664 is merged.

Is that alright or does that perhaps breaking changes that are not wanted?

Copy link
Contributor

Choose a reason for hiding this comment

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

@nc6 is working on this already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More than one update is fine. Yes we'll need to do it again to get the latest ledger changes.

@karknu karknu mentioned this pull request Jul 15, 2020
A couple tracers gained some extra warning fields.
@dcoutts
Copy link
Contributor Author

dcoutts commented Jul 15, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 15, 2020

👎 Rejected by too few approved reviews

Comment on lines 330 to 334
-- Text.concat [ "\nWarning: " <> formatText w o | w <- ws ]
--TODO: we could trace these warnings but then a
-- HasTextFormatter (LedgerWarning blk) constraint will propagate everywhere
-- and it's not clear that's how it's supposed to work, since we don't have any
-- other HasTextFormatter constraints floating around
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked at this in detail, but would show work for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dcoutts
Copy link
Contributor Author

dcoutts commented Jul 15, 2020

bors merge

iohk-bors bot added a commit that referenced this pull request Jul 15, 2020
1465: Update dependencies on consensus and ledger  r=dcoutts a=dcoutts

A couple tracers gained some extra warning fields. Log the new ledger
warnings

Only properly supported for the JSON logging, not for the human
readable, since I can't quite see how it's supposed to be done without
HasTextFormatter constaints propagating everywhere, which does not seem
to be the intended pattern.

Co-authored-by: Duncan Coutts <duncan@well-typed.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 15, 2020

Build failed

@karknu
Copy link
Contributor

karknu commented Jul 15, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 15, 2020

@iohk-bors iohk-bors bot merged commit 2658705 into master Jul 15, 2020
@iohk-bors iohk-bors bot deleted the dcoutts/update-deps branch July 15, 2020 13:54
iohk-bors bot added a commit that referenced this pull request Jul 15, 2020
1469: Blockfetch Config r=karknu a=karknu

Implements #1420 

Should be rebased when #1465 is merged.

Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>
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