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

refactor:Tidy up scalar module #251

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

winrhcp
Copy link
Contributor

@winrhcp winrhcp commented Oct 10, 2024

Rationale for this change
Mentioned in #234

What changes are included in this PR?
Combine any logic held within the four files "mont_scalar", "mont_scalar_from", "mont_scalar_test", "mont_scalar_from_test" into two files "mont_scalar" and "mont_scalar_test"

EDIT: this is part of #234, but does not complete it.

@JayWhite2357
Copy link
Contributor

Thanks for the PR!

It looks like the CI isn't passing. To expedite the review process, you can run the CI locally to ensure you fix the formatting/clippy issues: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#user-content-code-quality-checks.

@winrhcp
Copy link
Contributor Author

winrhcp commented Oct 10, 2024

Thanks for the PR!

It looks like the CI isn't passing. To expedite the review process, you can run the CI locally to ensure you fix the formatting/clippy issues: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#user-content-code-quality-checks.

Thank you for the feedback! I’ll run the CI checks locally again to address the formatting and Clippy issues as suggested. I’ll make sure everything is in order before resubmitting the PR.

Copy link
Contributor

@stuarttimwhite stuarttimwhite left a comment

Choose a reason for hiding this comment

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

Looks good!

@JayWhite2357 JayWhite2357 enabled auto-merge (squash) October 10, 2024 14:56
Copy link
Contributor

@JayWhite2357 JayWhite2357 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! Looks good!

@winrhcp just to be extra clear: the second task (Move the Scalar trait to its own "scalar" file within base/scalar.) of the #234 still needs to be done in order to complete the issue/bounty.

@JayWhite2357 JayWhite2357 merged commit 69e27c7 into spaceandtimelabs:main Oct 10, 2024
11 of 12 checks passed
Copy link

🎉 This PR is included in version 0.28.10 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@winrhcp winrhcp deleted the winrhcp-issue-234 branch October 10, 2024 18:15
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