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

Some simple Option / Result / iterator pattern simplifications #3205

Merged
merged 1 commit into from
Jan 29, 2020

Conversation

huitseeker
Copy link
Contributor

Makes a couple of places more readable. Opinions on these refactorings
loosely held, happy to accomodate review desiderata.


name: Pull Request
about: Pull Request checklist
title: ''
labels: ''
assignees: ''


If your PR is a work in progress, please feel free to create it and include a [WIP] tag in the PR name. We encourage everyone to PR early and often so that other developers know what you're working on.

Before submitting your PR for final review, please ensure that it:

  • Includes a proper description of what problems the PR addresses, as well as a detailed explanation as to what it changes
  • Explains whether/how the change is consensus breaking or breaks existing client functionality
  • Contains unit tests exercising new/changed functionality
  • Fully considers the potential impact of the change on other parts of the system
  • Describes how you've tested the change (e.g. against Floonet, etc)
  • Updates any documentation that's affected by the PR

Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

Had a quick scan through these and yes 👍.
Thanks for this!

Let me take a closer look over the next day or so and we can get this merged in.

Copy link
Member

@quentinlesceller quentinlesceller left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @huitseeker! Looking good, few comments about some structure (nothing blocking) but overall good "rustyfication" of the code :)

api/src/types.rs Show resolved Hide resolved
api/src/web.rs Show resolved Hide resolved
core/src/libtx/proof.rs Show resolved Hide resolved
pool/src/pool.rs Show resolved Hide resolved
@antiochp
Copy link
Member

Big 👍 on "Opinions on these refactorings loosely held" by the way. Really appreciate this!

@antiochp antiochp merged commit dcdbdd4 into mimblewimble:master Jan 29, 2020
@antiochp antiochp mentioned this pull request Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants