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

Allow brick/math 0.10 #424

Closed
wants to merge 1 commit into from
Closed

Allow brick/math 0.10 #424

wants to merge 1 commit into from

Conversation

remicollet
Copy link
Contributor

@remicollet remicollet commented Jun 20, 2022 •
edited
Loading

No description provided.

@remicollet remicollet requested a review from ramsey as a code owner June 20, 2022 08:45
@remicollet remicollet mentioned this pull request Jun 20, 2022
Closed
@remicollet
Copy link
Contributor Author

CI failure also happens on 8.1 with brick/math 0.9

@BenMorel
Copy link

Not sure why it fails with brick/math 0.9.x as 5.x passes. Let me know if I can help!

@remicollet
Copy link
Contributor Author

remicollet commented Jun 21, 2022 •
edited
Loading

Not sure why it fails with brick/math 0.9.x as 5.x passes. Let me know if I can help!

Explained in #425 ;) (so not related to brick/math version)

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Jun 22, 2022 •
edited
Loading

I think this would make sense on the 4.x branch? I also think the old moontoast/math could be removed I could not find any usage of it in the code base while tried creating the same PR: alexander-schranz@8659bb4

@ramsey
Copy link
Owner

ramsey commented Jun 22, 2022

Yes. This needs to be on the 4.x branch.

I also think the old moontoast/math could be removed I could not find any usage of it

I think I’m still using moontoast/math in some tests. If not, it can go away.

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Jun 22, 2022 •
edited
Loading

You are right Moontoast\Math\BigNumber is stilled used in the tests. Update could find a way to remove it: #426

@alexander-schranz alexander-schranz mentioned this pull request Jun 29, 2022
Closed
6 tasks
@GrahamCampbell
Copy link
Contributor

I've provided a PR to the v4 branch: #434.

@ramsey ramsey closed this in bd2df8d Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ramsey ramsey Awaiting requested review from ramsey ramsey 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.

None yet
5 participants
@remicollet @BenMorel @alexander-schranz @ramsey @GrahamCampbell