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.

Sign up for GitHub

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

Jump to bottom

add target_to_bits #16

Merged
merged 11 commits into from
Aug 7, 2024
Merged

add target_to_bits #16

merged 11 commits into from
Aug 7, 2024

Conversation

harsh-ps-2003
Copy link
Contributor

@harsh-ps-2003 harsh-ps-2003 commented Aug 1, 2024 •
edited
Loading

@vercel Vercel
Copy link

vercel bot commented Aug 1, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
raito ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2024 3:26pm

@maciejka
Copy link
Collaborator

maciejka commented Aug 1, 2024

Welcome and thanks for contribution.
Please add unit tests. And add comment to the issue #10 so I can assign you.

maciejka
maciejka reviewed Aug 2, 2024
View reviewed changes
Copy link
Collaborator

@maciejka maciejka left a comment

Choose a reason for hiding this comment

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

Where are the test cases?
I think it makes sense to squash #15 and #16 into a single pr.

@harsh-ps-2003 harsh-ps-2003 requested a review from maciejka August 5, 2024 06:11
@maciejka maciejka removed the request for review from b-j-roberts August 5, 2024 07:07
maciejka
maciejka requested changes Aug 5, 2024
View reviewed changes
tests/tests.cairo Outdated Show resolved Hide resolved
src/engine.cairo Outdated Show resolved Hide resolved
src/engine.cairo Outdated Show resolved Hide resolved
src/engine.cairo Outdated Show resolved Hide resolved
@maciejka maciejka mentioned this pull request Aug 5, 2024
Merged
3 tasks
@harsh-ps-2003 harsh-ps-2003 requested a review from m-kus as a code owner August 7, 2024 01:23
@harsh-ps-2003 harsh-ps-2003 requested a review from maciejka August 7, 2024 01:28
maciejka
maciejka requested changes Aug 7, 2024
View reviewed changes
src/utils.cairo Outdated Show resolved Hide resolved
@maciejka
Copy link
Collaborator

maciejka commented Aug 7, 2024

Please fix conflicts. You can put your functions into validation.cairo for now.

@harsh-ps-2003
Copy link
Contributor Author

harsh-ps-2003 commented Aug 7, 2024 •
edited
Loading

So I should put the target_to_bits and vice versa (with the helper functions) in validation.cairo for now. Basically dump all the changes into a single file? Or maybe keep a separate utils.cairo file? @maciejka

@maciejka
Copy link
Collaborator

maciejka commented Aug 7, 2024

So I should put the target_to_bits and vice versa (with the helper functions) in validation.cairo for now. Basically dump all the changes into a single file? Or maybe keep a separate utils.cairo file? @maciejka

keep utils separate

@maciejka
Copy link
Collaborator

maciejka commented Aug 7, 2024

Fix fmt and tests, please.

maciejka

This comment was marked as duplicate.

Sign in to view

maciejka
maciejka requested changes Aug 7, 2024
View reviewed changes
Copy link
Collaborator

@maciejka maciejka left a comment

Choose a reason for hiding this comment

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

There are still compilation errors

@maciejka maciejka merged commit 097fb81 into keep-starknet-strange:main Aug 7, 2024
5 checks passed
@harsh-ps-2003 harsh-ps-2003 deleted the target_to_bits branch August 8, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maciejka maciejka maciejka approved these changes

@m-kus m-kus Awaiting requested review from m-kus m-kus is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@harsh-ps-2003 @maciejka