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

feat: add fixed-size unsigned integer for ECLAIR #146

Merged
merged 32 commits into from
Jul 7, 2022
Merged

feat: add fixed-size unsigned integer for ECLAIR #146

merged 32 commits into from
Jul 7, 2022

Conversation

tsunrise
Copy link
Contributor

@tsunrise tsunrise commented Jun 29, 2022

Add AssertWithinRange trait and U128 type for ECLAIR.

Closes: #141


Before we can merge this PR, please make sure that all the following items have been checked off:

  • Linked to an issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one line describing your change in CHANGELOG.md and added the appropriate changelog label to the PR.
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Checked that changes and commits conform to the standards outlined in CONTRIBUTING.md.

@tsunrise tsunrise requested review from bhgomes and BoyuanFeng June 29, 2022 20:47
@tsunrise tsunrise added this to the v0.6.0 milestone Jun 29, 2022
@tsunrise tsunrise changed the title Fixed-Size Unsigned Integer for ECLAIR feat: Fixed-Size Unsigned Integer for ECLAIR Jun 29, 2022
@tsunrise tsunrise changed the title feat: Fixed-Size Unsigned Integer for ECLAIR feat: add fixed-xize unsigned integer for ECLAIR Jun 29, 2022
@BoyuanFeng BoyuanFeng added the changelog:added Changelog: add these changes to the `added` section of the changelog label Jun 30, 2022
@BoyuanFeng BoyuanFeng modified the milestones: v0.6.0, v0.5.3 Jun 30, 2022
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

@BoyuanFeng and I are going to rewrite some of this (and add some bit-decomposition optimizations).

@bhgomes bhgomes changed the title feat: add fixed-xize unsigned integer for ECLAIR feat: add fixed-size unsigned integer for ECLAIR Jul 1, 2022
@tsunrise
Copy link
Contributor Author

tsunrise commented Jul 2, 2022

@BoyuanFeng and I are going to rewrite some of this (and add some bit-decomposition optimizations).

Sounds good, thanks!

@tsunrise tsunrise requested a review from BoyuanFeng July 6, 2022 04:31
Copy link
Contributor

@bhgomes bhgomes left a comment

Choose a reason for hiding this comment

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

Some comments on the tests. Also, @BoyuanFeng don't forget the following:

  • Add generic implementation for UnsignedInteger from yesterday's discussion
  • Lets also add an unwrapping method for UnsignedInteger:
        pub fn into_inner(self) -> T {
            self.0
        }

@bhgomes bhgomes added P-high Priority: High A-openzl Area: Development Related to OpenZL and ECLAIR and removed norelease labels Jul 6, 2022
@tsunrise tsunrise requested review from BoyuanFeng and bhgomes July 6, 2022 20:39
bhgomes
bhgomes previously approved these changes Jul 6, 2022
BoyuanFeng
BoyuanFeng previously approved these changes Jul 6, 2022
@bhgomes bhgomes dismissed stale reviews from BoyuanFeng and themself via ab1d431 July 6, 2022 22:26
@bhgomes bhgomes requested review from BoyuanFeng and bhgomes July 6, 2022 22:26
@tsunrise tsunrise merged commit 087e6cf into main Jul 7, 2022
@tsunrise tsunrise deleted the feat/uint branch July 7, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-openzl Area: Development Related to OpenZL and ECLAIR changelog:added Changelog: add these changes to the `added` section of the changelog P-high Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Fixed-Size Unsigned Integer Interface to ECLAIR
3 participants