-
Notifications
You must be signed in to change notification settings - Fork 23
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
The Big Weeder Cleanup #1157
base: main
Are you sure you want to change the base?
The Big Weeder Cleanup #1157
Conversation
4f8aebf
to
58a3314
Compare
This is missing some few things already:
|
df66bdf
to
20d43ad
Compare
I added the no changelog label because it doesn't make sense to add a changelog fragment for the new packages as they are precisely new |
I don't think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! 🧹
@@ -0,0 +1,75 @@ | |||
cabal-version: 3.0 | |||
name: nf-vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, why is it called nf-vars
? 🤔 I guess it's due to "normal form". What about strict-vars
?
(newTMVar, newTMVarIO, traceTMVar, traceTMVarIO) | ||
import Control.Concurrent.Class.MonadSTM.TBQueue as LazySTM | ||
import Control.Concurrent.Class.MonadSTM.TQueue as LazySTM | ||
import Control.Concurrent.Class.MonadSTM.Strict.SVar hiding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could use this opportunity to import explicitly.
@@ -116,7 +116,7 @@ data instance Block.StorageConfig TestBlock = TestBlockStorageConfig | |||
Mempool support | |||
-------------------------------------------------------------------------------} | |||
|
|||
newtype instance Ledger.GenTx TestBlock = TestBlockGenTx { unGenTx :: Tx } | |||
newtype instance Ledger.GenTx TestBlock = TestBlockGenTx Tx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not unlikely that we might want to use this function at some point, so I don't necessarily agree with weeder's suggestion here, especially considering how little code we're removing.
@@ -174,7 +174,7 @@ instance CanHardFork xs | |||
|
|||
applyBlockLedgerResult cfg | |||
(HardForkBlock (OneEraBlock block)) | |||
(TickedHardForkLedgerState transition st) = | |||
thfls = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the previous version more readable?
I couldn't see any references for these in cardano-{api,cli,node}, and even if, they should be easy to replace.
Description
A big cleanup of unused functions plus moving the resource registry and the normalform vars into separate packages