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

Added the separated big_decimal_ext #196

Closed

Conversation

pulkitgovrani
Copy link

@pulkitgovrani pulkitgovrani commented Sep 29, 2024

/claim #113

Suggested Solution:
Creating a new big_decimal_ext file in the math section inside proof_of_sql folder & pushing the code conversion there.

Problem Faced by me while implementing above solution:
I was getting circular dependency error while doing cargo build because proof-of-sql would become dependent on proof-of-sql-parser and vice versa.

Therefore, to overcome this problem I created a new folder proof-of-sql-utils containing the file code required by both other modules.

Please do share the feedback

Copy link

algora-pbc bot commented Sep 29, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

@JayWhite2357
Copy link
Contributor

/claim #113

Suggested Solution: Creating a new big_decimal_ext file in the math section inside proof_of_sql folder & pushing the code conversion there.

Problem Faced by me while implementing above solution: I was getting circular dependency error while doing cargo build because proof-of-sql would become dependent on proof-of-sql-parser and vice versa.

Therefore, to overcome this problem I created a new folder proof-of-sql-utils containing the file code required by both other modules.

Please do share the feedback

It doesn't look like proof-of-sql-parser depends on proof-of-sql-utils at all as you have it written.
To me this implies that it would be fine putting it in the math section without any circular dependencies since you wouldn't need proof-of-sql-parser to depend on proof-of-sql.

Am I missing something here?

@JayWhite2357 JayWhite2357 marked this pull request as draft October 2, 2024 02:03
@JayWhite2357 JayWhite2357 removed the request for review from Dustin-Ray October 2, 2024 02:04
@JayWhite2357
Copy link
Contributor

@pulkitgovrani I believe that this PR is almost complete. It was very close and just needs a bit of updating. Is there any clarification you need at this point?
Outside of possibly minor changes, I think the only thing that needs to get done is resolving conflicts with main and resolving this comment: #196 (comment).

@JayWhite2357
Copy link
Contributor

@pulkitgovrani Closing because I don't see activity on this. Feel free to reopen when it becomes ready.

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.

2 participants