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

Expose ability to configure mempoolCapacityOverride #3413

Merged

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Oct 6, 2021

No description provided.

@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch from cddd6dd to 07ba981 Compare October 6, 2021 12:33
@newhoggy newhoggy marked this pull request as ready for review October 6, 2021 12:33
@newhoggy newhoggy requested a review from nfrisby as a code owner October 6, 2021 12:33
nfrisby
nfrisby previously requested changes Oct 6, 2021
Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

Thanks for the proactive PR. I Requested Changes because there's a bug in the current commit.

ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch from 07ba981 to c785315 Compare October 8, 2021 09:51
@newhoggy newhoggy requested a review from nfrisby October 8, 2021 09:52
@newhoggy newhoggy dismissed nfrisby’s stale review October 9, 2021 04:02

Comments addressed

Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

One last round of Request Changes, both very specific and straight-forward. Otherwise it looks great!

ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch 2 times, most recently from 1596428 to ef9eb4d Compare October 12, 2021 06:49
@newhoggy newhoggy dismissed nfrisby’s stale review October 12, 2021 06:50

Comments addressed.

@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch 4 times, most recently from 56ebc21 to 8459495 Compare October 12, 2021 09:18
Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

Approved, and asked for one last minor fixup. Thanks!

ouroboros-consensus/src/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
@nfrisby
Copy link
Contributor

nfrisby commented Oct 12, 2021

I don't know what that buildkite failure is about. Hopefully ephemeral.

DistributionziTypesziVersionRangeziInternal_zdwversionRangeParser_closure'
`cc' failed in phase `Linker'. (Exit code: 1)```

@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch from 8459495 to e220de8 Compare October 12, 2021 22:36
@newhoggy
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Oct 13, 2021
3413: Expose ability to configure mempoolCapacityOverride r=newhoggy a=newhoggy



Co-authored-by: John Ky <john.ky@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 13, 2021

Build failed:

@newhoggy newhoggy force-pushed the expose-ability-to-configure-mempool-capacity-override branch from e220de8 to e019aff Compare October 13, 2021 07:13
@newhoggy
Copy link
Contributor Author

bors merge

@iohk-bors iohk-bors bot merged commit e72d3dc into master Oct 13, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 13, 2021

@iohk-bors iohk-bors bot deleted the expose-ability-to-configure-mempool-capacity-override branch October 13, 2021 07:59
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.

2 participants